Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUGFIX: Make URI suffix configurable for the ESFrontendNode RoutePart #5224

Merged
merged 1 commit into from
Aug 28, 2024

Conversation

nezaniel
Copy link
Member

This makes the uri suffix (e.g. .html) configurable on route (part) level, which is essential for Neos.SEO's XML sitemap and robots.txt as well as custom formats for rendering nodes as pdf etc.

Upgrade instructions

none

Review instructions

none

Checklist

  • Code follows the PSR-12 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

@github-actions github-actions bot added the 9.0 label Aug 27, 2024
@nezaniel nezaniel changed the title Male URI suffix configurable for the ESFrontendNode RoutePart Make URI suffix configurable for the ESFrontendNode RoutePart Aug 27, 2024
@dlubitz
Copy link
Contributor

dlubitz commented Aug 27, 2024

Looks like a duplicate of #5189

@nezaniel
Copy link
Member Author

yes, but without the other changes

Copy link
Member

@mhsdesign mhsdesign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good ;)

it seems this got just lost in the rewrite.
In Neos 8 it looked like such:

protected function truncateUriPathSuffix(string $uriPath)

We were only using the options as it was only a single cr and we configured the default via defaultUriSuffix globally.

@mhsdesign mhsdesign changed the title Make URI suffix configurable for the ESFrontendNode RoutePart BUGFIX: Make URI suffix configurable for the ESFrontendNode RoutePart Aug 28, 2024
@github-actions github-actions bot added the Bug label Aug 28, 2024
@mhsdesign
Copy link
Member

But this is only half of the fix!!!

in 8.3 we also have other options aside from uriPathSuffix which are used in Neos Seo!!! https://github.com/neos/neos-seo/blob/c2205573caac5f6a2b37dec4651ad0d336b4f31d/Configuration/Routes.yaml#L18

onlyMatchSiteNodes:

  • Whether the current route part should only match/resolve site nodes (e.g. the homepage)

nodeType:

  • Whether the given $node is allowed according to the "nodeType" option

Copy link
Member

@mhsdesign mhsdesign left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above, its only 50% of the rent ...

@nezaniel
Copy link
Member Author

we ignored onlyMatchSiteNodes for now because my-site/some-path/robots.txt does not 404 as expected but does not lie either. It would require a larger effort to implement it and is postponed to later

As with suffix vs. CR: we discussed this and came to the conclusion that we are fine with global overrides on route level for now, as CR specific overrides are no requirement yet. This as well can be added later if necessary.

In conclusion: This change is already sufficient for Neos.Seo 4.1.4

@nezaniel nezaniel requested a review from mhsdesign August 28, 2024 14:09
@mhsdesign
Copy link
Member

mhsdesign commented Aug 28, 2024

Perfekt, i just wanted it to bring that to attention. Then lets have it. Fine by reading ;)

@nezaniel nezaniel merged commit cfa3a92 into 9.0 Aug 28, 2024
12 checks passed
@nezaniel nezaniel deleted the configurableUriSuffix branch August 28, 2024 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants