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

Sections URL should not be generated in sitemap #473

Closed
David-Crty opened this issue Mar 20, 2023 · 3 comments · Fixed by #475
Closed

Sections URL should not be generated in sitemap #473

David-Crty opened this issue Mar 20, 2023 · 3 comments · Fixed by #475

Comments

@David-Crty
Copy link

David-Crty commented Mar 20, 2023

Because of this file, we are generating a cms_sections XML sitemap with URLs like /{_locale}/section/{code}.
When you try to access the URL, you get a 500 because there is no $template variable. You get an error like Unable to find template "/show.html.twig" (looked into: /app/vendor/knplabs/knp-menu/src/Knp/Menu/Resources/views, /app/templates, /app/templates, /app/vendor/symfony/twig-bridge/Resources/views/Form).

IMHO, we should not generate section URLs in the sitemap, as the doc says. The sections look to be rendered using TWIG, e.g.: {{ render(path('bitbag_sylius_cms_plugin_shop_block_index_by_section_code', {'sectionCode' : 'blog', 'template' : '@BitBagSyliusCmsPlugin/Shop/Block/index.html.twig'})) }}

I could make a PR removing the URL section generation in the sitemap; what do you think?

For now, to avoid thous URLs to be added in the sitemap, I did a $container->removeDefinition('bitbag_sylius_cms_plugin.sitemap_provider.section');

@liszkapawel
Copy link
Contributor

Hello @David-Crty,
If you are able to prepare a pull request, I will be grateful :)

@MacBalc
Copy link
Contributor

MacBalc commented May 12, 2023

@David-Crty could you take a look at PR i have prepped? Is this something addressing your issue?

@David-Crty
Copy link
Author

David-Crty commented May 23, 2023

@David-Crty could you take a look at PR i have prepped? Is this something addressing your issue?

Yes this will fix the issue, thanks for your PR ❤️

I will close this issue when #475 is merged 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants