From 1230e400c685a901f2e744d68c934f3a02d79add Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Bia=C5=82czak?= Date: Mon, 7 Apr 2025 09:39:50 +0200 Subject: [PATCH 1/5] IBX-6554: Fixed (location-view), prevent Twig errors when location is not defined --- .../admin/content/location_view.html.twig | 74 ++++++++++--------- 1 file changed, 39 insertions(+), 35 deletions(-) diff --git a/src/bundle/Resources/views/themes/admin/content/location_view.html.twig b/src/bundle/Resources/views/themes/admin/content/location_view.html.twig index aeaa7fcc47..0a87b6f107 100644 --- a/src/bundle/Resources/views/themes/admin/content/location_view.html.twig +++ b/src/bundle/Resources/views/themes/admin/content/location_view.html.twig @@ -1,5 +1,3 @@ -{% set location_path = location.pathString %} - {% extends "@ibexadesign/ui/layout.html.twig" %} {% trans_default_domain 'ibexa_locationview' %} @@ -29,7 +27,7 @@ {{ ibexa_render_component_group('content-tree-before') }}
@@ -57,7 +55,9 @@ {% endblock %} {% block context_menu %} - {% set content_sidebar_right = knp_menu_get('ezplatform_admin_ui.menu.content.sidebar_right', [], {'location': location, 'content': content, 'content_type': content_type}) %} + {% set content_sidebar_right = location is defined and location is not null + ? knp_menu_get('ezplatform_admin_ui.menu.content.sidebar_right', [], {'location': location, 'content': content, 'content_type': content_type}) + : [] %} {{ knp_menu_render(content_sidebar_right, {'template': '@ibexadesign/ui/menu/context_menu.html.twig'}) }}
@@ -109,23 +109,25 @@ {% endblock %} {% endembed %} - {% if location.hidden or location.invisible %} -
- {% include '@ibexadesign/ui/component/alert/alert.html.twig' with { - type: 'info', - title: 'content.hidden.message'|trans()|desc('This Content item or its Location is hidden.'), - icon_path: ibexa_icon_path('hide'), - class: 'mb-4', - } only %} -
+ {% if location is defined and location is not null %} + {% if location.hidden or location.invisible %} +
+ {% include '@ibexadesign/ui/component/alert/alert.html.twig' with { + type: 'info', + title: 'content.hidden.message'|trans()|desc('This Content item or its Location is hidden.'), + icon_path: ibexa_icon_path('hide'), + class: 'mb-4', + } only %} +
+ {% endif %} + {{ ibexa_render_component_group( + 'location-view-content-alerts', + { + 'versionInfo': content.versionInfo, + 'location': location, + } + ) }} {% endif %} - {{ ibexa_render_component_group( - 'location-view-content-alerts', - { - 'versionInfo': content.versionInfo, - 'location': location, - } - ) }} {% endblock %} {% block content %} @@ -134,20 +136,22 @@
{# 'is_location_visible': location.invisible - param deprecated since EZP-32395 #} - {{ ibexa_render_component_group('location-view-tab-groups', { - 'content': content, - 'location': location, - 'contentType': content_type, - 'draft_pagination_params': draft_pagination_params, - 'reverse_relation_pagination_params': reverse_relation_pagination_params, - 'relation_pagination_params': relation_pagination_params, - 'custom_urls_pagination_params': custom_urls_pagination_params, - 'system_urls_pagination_params': system_urls_pagination_params, - 'roles_pagination_params': roles_pagination_params, - 'policies_pagination_params': policies_pagination_params, - 'is_location_visible': not location.invisible, - 'subitems_module': subitems_module, - }) }} + {% if location is defined and location is not null %} + {{ ibexa_render_component_group('location-view-tab-groups', { + 'content': content, + 'location': location, + 'contentType': content_type, + 'draft_pagination_params': draft_pagination_params, + 'reverse_relation_pagination_params': reverse_relation_pagination_params, + 'relation_pagination_params': relation_pagination_params, + 'custom_urls_pagination_params': custom_urls_pagination_params, + 'system_urls_pagination_params': system_urls_pagination_params, + 'roles_pagination_params': roles_pagination_params, + 'policies_pagination_params': policies_pagination_params, + 'is_location_visible': not location.invisible, + 'subitems_module': subitems_module, + }) }} + {% endif %} {% if content_type.isContainer %} {{ form_start(form_subitems_content_edit, { 'action': path('ibexa.content.edit'), 'attr': { 'hidden': 'hidden' }}) }} @@ -165,7 +169,7 @@
- {% if content_has_reverse_relations and not location.contentInfo.isHidden %} + {% if content_has_reverse_relations and location is defined and location is not null and not location.contentInfo.isHidden %} {% include '@ibexadesign/content/modal/hide_confirmation.html.twig' %} {% endif %}
From 03d9179324673750b48640f7526e313a6b07e961 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Bia=C5=82czak?= Date: Wed, 4 Jun 2025 08:27:48 +0200 Subject: [PATCH 2/5] IBX-6554: Fixed (location-view), prevent Twig errors when location is not defined # Conflicts: # src/bundle/Resources/views/themes/admin/content/location_view.html.twig --- .../admin/content/location_view.html.twig | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/src/bundle/Resources/views/themes/admin/content/location_view.html.twig b/src/bundle/Resources/views/themes/admin/content/location_view.html.twig index e90ae1d249..94285017fa 100644 --- a/src/bundle/Resources/views/themes/admin/content/location_view.html.twig +++ b/src/bundle/Resources/views/themes/admin/content/location_view.html.twig @@ -1,5 +1,3 @@ -{% set location_path = location.pathString %} - {% extends "@ibexadesign/ui/layout.html.twig" %} {% trans_default_domain 'ibexa_locationview' %} @@ -29,7 +27,7 @@ {{ ibexa_twig_component_group('admin-ui-content-tree-before') }}
@@ -57,7 +55,9 @@ {% endblock %} {% block context_menu %} - {% set content_sidebar_right = knp_menu_get('ezplatform_admin_ui.menu.content.sidebar_right', [], {'location': location, 'content': content, 'content_type': content_type}) %} + {% set content_sidebar_right = location is defined and location is not null + ? knp_menu_get('ezplatform_admin_ui.menu.content.sidebar_right', [], {'location': location, 'content': content, 'content_type': content_type}) + : [] %} {{ knp_menu_render(content_sidebar_right, {'template': '@ibexadesign/ui/menu/context_menu.html.twig'}) }}
@@ -109,6 +109,7 @@ {% endblock %} {% endembed %} + {% if location is defined and location is not null %} {% if location.hidden or location.invisible %}
{% include '@ibexadesign/ui/component/alert/alert.html.twig' with { @@ -127,26 +128,29 @@ } ) }} {% endblock %} +{% endblock %} {% block content %}
- {{ ibexa_twig_component_group('admin-ui-location-view-tab-groups', { - 'content': content, - 'location': location, - 'contentType': content_type, - 'draft_pagination_params': draft_pagination_params, - 'reverse_relation_pagination_params': reverse_relation_pagination_params, - 'relation_pagination_params': relation_pagination_params, - 'custom_urls_pagination_params': custom_urls_pagination_params, - 'system_urls_pagination_params': system_urls_pagination_params, - 'roles_pagination_params': roles_pagination_params, - 'policies_pagination_params': policies_pagination_params, - 'is_location_visible': not location.invisible, - 'subitems_module': subitems_module, - }) }} + {% if location is defined and location is not null %} + {{ ibexa_render_component_group('admin-ui-location-view-tab-groups', { + 'content': content, + 'location': location, + 'contentType': content_type, + 'draft_pagination_params': draft_pagination_params, + 'reverse_relation_pagination_params': reverse_relation_pagination_params, + 'relation_pagination_params': relation_pagination_params, + 'custom_urls_pagination_params': custom_urls_pagination_params, + 'system_urls_pagination_params': system_urls_pagination_params, + 'roles_pagination_params': roles_pagination_params, + 'policies_pagination_params': policies_pagination_params, + 'is_location_visible': not location.invisible, + 'subitems_module': subitems_module, + }) }} + {% endif %} {% if content_type.isContainer %} {{ form_start(form_subitems_content_edit, { 'action': path('ibexa.content.edit'), 'attr': { 'hidden': 'hidden' }}) }} @@ -164,7 +168,7 @@
- {% if content_has_reverse_relations and not location.contentInfo.isHidden %} + {% if content_has_reverse_relations and location is defined and location is not null and not location.contentInfo.isHidden %} {% include '@ibexadesign/content/modal/hide_confirmation.html.twig' %} {% endif %}
From 3fb2a14df6b221a19635ae322a1d431505965324 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Bia=C5=82czak?= Date: Mon, 16 Jun 2025 10:49:49 +0200 Subject: [PATCH 3/5] IBX-6554: Added null checks to prevent errors in LocationVoter and Twig templates --- phpstan-baseline.neon | 6 --- .../admin/content/location_view.html.twig | 40 +++++++++++++------ src/lib/Menu/Voter/LocationVoter.php | 24 +++++------ 3 files changed, 38 insertions(+), 32 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 4a49247916..41c51e2d40 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -11502,12 +11502,6 @@ parameters: count: 1 path: src/lib/Menu/Voter/LocationVoter.php - - - message: '#^Cannot access property \$id on Ibexa\\Contracts\\Core\\Repository\\Values\\Content\\Location\|null\.$#' - identifier: property.nonObject - count: 1 - path: src/lib/Menu/Voter/LocationVoter.php - - message: '#^Call to an undefined method Symfony\\Component\\HttpFoundation\\Session\\SessionInterface\:\:getFlashBag\(\)\.$#' identifier: method.notFound diff --git a/src/bundle/Resources/views/themes/admin/content/location_view.html.twig b/src/bundle/Resources/views/themes/admin/content/location_view.html.twig index 41653e9d7e..08dc7889fc 100644 --- a/src/bundle/Resources/views/themes/admin/content/location_view.html.twig +++ b/src/bundle/Resources/views/themes/admin/content/location_view.html.twig @@ -37,7 +37,7 @@ {% block breadcrumbs %} {% set items = [] %} - {% for path_location in path_locations %} + {% for path_location in path_locations ?? [] %} {% if not loop.last %} {% set items = items|merge([{ 'value': ibexa_content_name(path_location.contentInfo), @@ -55,13 +55,19 @@ {% endblock %} {% block context_menu %} - {% set content_sidebar_right = location is defined and location is not null - ? knp_menu_get('ezplatform_admin_ui.menu.content.sidebar_right', [], {'location': location, 'content': content, 'content_type': content_type}) - : [] %} - {{ knp_menu_render(content_sidebar_right, {'template': '@ibexadesign/ui/menu/context_menu.html.twig'}) }} + {% if location is defined and location is not null %} + {% set content_sidebar_right = knp_menu_get('ezplatform_admin_ui.menu.content.sidebar_right', [], { + 'location': location, + 'content': content, + 'content_type': content_type + }) %} + {{ knp_menu_render(content_sidebar_right, {'template': '@ibexadesign/ui/menu/context_menu.html.twig'}) }} + {% endif %}
- {% include '@ibexadesign/content/widget/content_create.html.twig' with {'form': form_content_create, content } only %} + {% if form_content_create is defined %} + {% include '@ibexadesign/content/widget/content_create.html.twig' with {'form': form_content_create, content } only %} + {% endif %} {% if form_content_edit is defined and form_user_edit is not defined %} {% include '@ibexadesign/content/widget/content_edit.html.twig' with {'form': form_content_edit} only %} {% endif %} @@ -78,10 +84,18 @@ {% if form_user_delete is defined %} {% include '@ibexadesign/content/modal/user_delete.html.twig' with {'form': form_user_delete} only %} {% endif %} - {{ form(form_location_copy, {'action': path('ibexa.location.copy')}) }} - {{ form(form_location_move, {'action': path('ibexa.location.move')}) }} - {{ form(form_location_copy_subtree, {'action': path('ibexa.location.copy_subtree')}) }} - {{ form(form_content_visibility_update, {'action': path('ibexa.content.update_visibility')}) }} + {% if form_location_copy is defined %} + {{ form(form_location_copy, {'action': path('ibexa.location.copy')}) }} + {% endif %} + {% if form_location_move is defined %} + {{ form(form_location_move, {'action': path('ibexa.location.move')}) }} + {% endif %} + {% if form_location_copy_subtree is defined %} + {{ form(form_location_copy_subtree, {'action': path('ibexa.location.copy_subtree')}) }} + {% endif %} + {% if form_content_visibility_update is defined %} + {{ form(form_content_visibility_update, {'action': path('ibexa.content.update_visibility')}) }} + {% endif %} {% endblock %} {% block header %} @@ -168,9 +182,9 @@
- {% if content_has_reverse_relations and location is defined and location is not null and not location.contentInfo.isHidden %} - {% include '@ibexadesign/content/modal/hide_confirmation.html.twig' %} - {% endif %} + {% if content_has_reverse_relations is defined and content_has_reverse_relations and location is defined and location is not null and not location.contentInfo.isHidden %} + {% include '@ibexadesign/content/modal/hide_confirmation.html.twig' %} + {% endif %}
{% endblock %} diff --git a/src/lib/Menu/Voter/LocationVoter.php b/src/lib/Menu/Voter/LocationVoter.php index fa8a0fefd6..79816932d6 100644 --- a/src/lib/Menu/Voter/LocationVoter.php +++ b/src/lib/Menu/Voter/LocationVoter.php @@ -17,22 +17,13 @@ class LocationVoter implements VoterInterface { private const CONTENT_VIEW_ROUTE_NAME = 'ibexa.content.view'; - /** - * @var \Symfony\Component\HttpFoundation\RequestStack - */ - private $requestStack; - - /** - * @param \Symfony\Component\HttpFoundation\RequestStack $requestStack - */ + private RequestStack $requestStack; + public function __construct(RequestStack $requestStack) { $this->requestStack = $requestStack; } - /** - * {@inheritdoc} - */ public function matchItem(ItemInterface $item): ?bool { $routes = $item->getExtra('routes', []); @@ -43,8 +34,15 @@ public function matchItem(ItemInterface $item): ?bool $contentView = $request->attributes->get('view'); $locationId = $route['parameters']['locationId']; - if ($contentView instanceof ContentView && in_array($locationId, $contentView->getLocation()->path ?? [$contentView->getLocation()->id])) { - return true; + if ($contentView instanceof ContentView) { + $location = $contentView->getLocation(); + if ($location !== null) { + $path = $location->path ?? [$location->id]; + + if (in_array($locationId, $path, true)) { + return true; + } + } } } } From 0f595d0ef60a8531bf2f193f898a2468a0a65eee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Bia=C5=82czak?= Date: Mon, 16 Jun 2025 13:07:07 +0200 Subject: [PATCH 4/5] IBX-6554: Removed redundant null checks and added validation for missing location in controller --- .../Controller/ContentViewController.php | 7 ++ .../admin/content/location_view.html.twig | 98 ++++++++----------- 2 files changed, 49 insertions(+), 56 deletions(-) diff --git a/src/bundle/Controller/ContentViewController.php b/src/bundle/Controller/ContentViewController.php index 4df7bc95b0..11efb514ce 100644 --- a/src/bundle/Controller/ContentViewController.php +++ b/src/bundle/Controller/ContentViewController.php @@ -41,6 +41,7 @@ use Ibexa\Contracts\Core\Repository\Values\Content\Location; use Ibexa\Contracts\Core\Repository\Values\Content\VersionInfo; use Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface; +use Ibexa\Core\Base\Exceptions\NotFoundException; use Ibexa\Core\MVC\Symfony\Locale\UserLanguagePreferenceProviderInterface; use Ibexa\Core\MVC\Symfony\View\ContentView; use Symfony\Component\Form\FormFactoryInterface; @@ -158,6 +159,12 @@ public function __construct( */ public function locationViewAction(Request $request, ContentView $view): ContentView { + $location = $view->getLocation(); + if ($location === null || $location->id === null) { + $contentId = $view->getContent()->id ?? 'unknown'; + throw new NotFoundException('Location', "content ID {$contentId}"); + } + // We should not cache ContentView because we use forms with CSRF tokens in template // JIRA ref: https://issues.ibexa.co/browse/EZP-28190 $view->setCacheEnabled(false); diff --git a/src/bundle/Resources/views/themes/admin/content/location_view.html.twig b/src/bundle/Resources/views/themes/admin/content/location_view.html.twig index 08dc7889fc..82300c234f 100644 --- a/src/bundle/Resources/views/themes/admin/content/location_view.html.twig +++ b/src/bundle/Resources/views/themes/admin/content/location_view.html.twig @@ -37,7 +37,7 @@ {% block breadcrumbs %} {% set items = [] %} - {% for path_location in path_locations ?? [] %} + {% for path_location in path_locations %} {% if not loop.last %} {% set items = items|merge([{ 'value': ibexa_content_name(path_location.contentInfo), @@ -55,14 +55,12 @@ {% endblock %} {% block context_menu %} - {% if location is defined and location is not null %} - {% set content_sidebar_right = knp_menu_get('ezplatform_admin_ui.menu.content.sidebar_right', [], { - 'location': location, - 'content': content, - 'content_type': content_type - }) %} - {{ knp_menu_render(content_sidebar_right, {'template': '@ibexadesign/ui/menu/context_menu.html.twig'}) }} - {% endif %} + {% set content_sidebar_right = knp_menu_get('ezplatform_admin_ui.menu.content.sidebar_right', [], { + 'location': location, + 'content': content, + 'content_type': content_type + }) %} + {{ knp_menu_render(content_sidebar_right, {'template': '@ibexadesign/ui/menu/context_menu.html.twig'}) }}
{% if form_content_create is defined %} @@ -84,18 +82,10 @@ {% if form_user_delete is defined %} {% include '@ibexadesign/content/modal/user_delete.html.twig' with {'form': form_user_delete} only %} {% endif %} - {% if form_location_copy is defined %} - {{ form(form_location_copy, {'action': path('ibexa.location.copy')}) }} - {% endif %} - {% if form_location_move is defined %} - {{ form(form_location_move, {'action': path('ibexa.location.move')}) }} - {% endif %} - {% if form_location_copy_subtree is defined %} - {{ form(form_location_copy_subtree, {'action': path('ibexa.location.copy_subtree')}) }} - {% endif %} - {% if form_content_visibility_update is defined %} - {{ form(form_content_visibility_update, {'action': path('ibexa.content.update_visibility')}) }} - {% endif %} + {{ form(form_location_copy, {'action': path('ibexa.location.copy')}) }} + {{ form(form_location_move, {'action': path('ibexa.location.move')}) }} + {{ form(form_location_copy_subtree, {'action': path('ibexa.location.copy_subtree')}) }} + {{ form(form_content_visibility_update, {'action': path('ibexa.content.update_visibility')}) }} {% endblock %} {% block header %} @@ -123,25 +113,23 @@ {% endblock %} {% endembed %} - {% if location is defined and location is not null %} - {% if location.hidden or location.invisible %} -
- {% include '@ibexadesign/ui/component/alert/alert.html.twig' with { - type: 'info', - title: 'content.hidden.message'|trans()|desc('This Content item or its Location is hidden.'), - icon_path: ibexa_icon_path('hide'), - class: 'mb-4', - } only %} -
- {% endif %} - {{ ibexa_render_component_group( - 'admin-ui-location-view-content-alerts', - { - 'versionInfo': content.versionInfo, - 'location': location, - } - ) }} + {% if location.hidden or location.invisible %} +
+ {% include '@ibexadesign/ui/component/alert/alert.html.twig' with { + type: 'info', + title: 'content.hidden.message'|trans()|desc('This Content item or its Location is hidden.'), + icon_path: ibexa_icon_path('hide'), + class: 'mb-4', + } only %} +
{% endif %} + {{ ibexa_twig_component_group( + 'admin-ui-location-view-content-alerts', + { + 'versionInfo': content.versionInfo, + 'location': location, + } + ) }} {% endblock %} {% block content %} @@ -149,22 +137,20 @@
- {% if location is defined and location is not null %} - {{ ibexa_render_component_group('admin-ui-location-view-tab-groups', { - 'content': content, - 'location': location, - 'contentType': content_type, - 'draft_pagination_params': draft_pagination_params, - 'reverse_relation_pagination_params': reverse_relation_pagination_params, - 'relation_pagination_params': relation_pagination_params, - 'custom_urls_pagination_params': custom_urls_pagination_params, - 'system_urls_pagination_params': system_urls_pagination_params, - 'roles_pagination_params': roles_pagination_params, - 'policies_pagination_params': policies_pagination_params, - 'is_location_visible': not location.invisible, - 'subitems_module': subitems_module, - }) }} - {% endif %} + {{ ibexa_twig_component_group('admin-ui-location-view-tab-groups', { + 'content': content, + 'location': location, + 'contentType': content_type, + 'draft_pagination_params': draft_pagination_params, + 'reverse_relation_pagination_params': reverse_relation_pagination_params, + 'relation_pagination_params': relation_pagination_params, + 'custom_urls_pagination_params': custom_urls_pagination_params, + 'system_urls_pagination_params': system_urls_pagination_params, + 'roles_pagination_params': roles_pagination_params, + 'policies_pagination_params': policies_pagination_params, + 'is_location_visible': not location.invisible, + 'subitems_module': subitems_module, + }) }} {% if content_type.isContainer %} {{ form_start(form_subitems_content_edit, { 'action': path('ibexa.content.edit'), 'attr': { 'hidden': 'hidden' }}) }} @@ -182,7 +168,7 @@
- {% if content_has_reverse_relations is defined and content_has_reverse_relations and location is defined and location is not null and not location.contentInfo.isHidden %} + {% if content_has_reverse_relations and not location.contentInfo.isHidden %} {% include '@ibexadesign/content/modal/hide_confirmation.html.twig' %} {% endif %}
From 41923091fefc84cfc20f9b02c456d5bd35c6c9c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Bia=C5=82czak?= Date: Wed, 18 Jun 2025 14:35:47 +0200 Subject: [PATCH 5/5] IBX-6554: Refactored location and content property accessors for better consistency and null safety --- src/bundle/Controller/ContentViewController.php | 6 +++--- src/lib/Menu/Voter/LocationVoter.php | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/bundle/Controller/ContentViewController.php b/src/bundle/Controller/ContentViewController.php index 11efb514ce..c6d9199d59 100644 --- a/src/bundle/Controller/ContentViewController.php +++ b/src/bundle/Controller/ContentViewController.php @@ -160,8 +160,8 @@ public function __construct( public function locationViewAction(Request $request, ContentView $view): ContentView { $location = $view->getLocation(); - if ($location === null || $location->id === null) { - $contentId = $view->getContent()->id ?? 'unknown'; + if ($location === null) { + $contentId = $view->getContent()->getId(); throw new NotFoundException('Location', "content ID {$contentId}"); } @@ -169,7 +169,7 @@ public function locationViewAction(Request $request, ContentView $view): Content // JIRA ref: https://issues.ibexa.co/browse/EZP-28190 $view->setCacheEnabled(false); - if (!$view->getContent()->contentInfo->isTrashed()) { + if (!$view->getContent()->getContentInfo()->isTrashed()) { $this->supplyPathLocations($view); $this->subitemsContentViewParameterSupplier->supply($view); $this->supplyContentActionForms($view); diff --git a/src/lib/Menu/Voter/LocationVoter.php b/src/lib/Menu/Voter/LocationVoter.php index 79816932d6..7dcbb2b39e 100644 --- a/src/lib/Menu/Voter/LocationVoter.php +++ b/src/lib/Menu/Voter/LocationVoter.php @@ -37,7 +37,7 @@ public function matchItem(ItemInterface $item): ?bool if ($contentView instanceof ContentView) { $location = $contentView->getLocation(); if ($location !== null) { - $path = $location->path ?? [$location->id]; + $path = $location->getPath(); if (in_array($locationId, $path, true)) { return true;