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

Symfony flex structure : templates override #9719

Closed
wadjeroudi opened this issue Sep 20, 2018 · 2 comments
Closed

Symfony flex structure : templates override #9719

wadjeroudi opened this issue Sep 20, 2018 · 2 comments
Labels
Documentation Documentation related issues and PRs - requests, fixes, proposals. DX Issues and PRs aimed at improving Developer eXperience.

Comments

@wadjeroudi
Copy link
Contributor

wadjeroudi commented Sep 20, 2018

Sylius version affected: 1.3-DEV

Description

When you use new symfony flex with sylius, you can't override templates using this method
https://symfony.com/doc/current/bundles/override.html#templates

For example if you want to rewrite SyliusAdminBundle/Channel/_form.html.twig, you should be able to put your file in templates/bundles/SyliusAdminBundle/Channel/_form.html.twig

Possible Solution
This issue and proposed fix is relevant #8929 but not totally.
See symfony/symfony#25661

templates:
  form: SyliusAdminBundle:Channel:_form.html.twig

should be

templates:
  form: @SyliusAdmin/Channel/_form.html.twig
@wadjeroudi
Copy link
Contributor Author

    sylius.theme.templating.name_parser:
        class: Symfony\Bundle\FrameworkBundle\Templating\TemplateNameParser
        arguments: ['@kernel']
  • For the themes templates to work in the new structure, we need to fix the sylius.theme.locator.bundle_resource, here is what I use, but it works only with the new structure (no Resources folder) :
class ThemeBundleResourceLocator implements ResourceLocatorInterface
{
    /**
     * @var Filesystem
     */
    private $filesystem;

    /**
     * @var KernelInterface
     */
    private $kernel;

    /**
     * @param Filesystem $filesystem
     * @param KernelInterface $kernel
     */
    public function __construct(Filesystem $filesystem, KernelInterface $kernel)
    {
        $this->filesystem = $filesystem;
        $this->kernel = $kernel;
    }

    /**
     * {@inheritdoc}
     *
     * @param string $resourcePath Eg. "@Acme/template.html.twig"
     */
    public function locateResource(string $resourcePath, ThemeInterface $theme): string
    {
        $this->assertResourcePathIsValid($resourcePath, $theme);

        $bundleName = $this->getBundleNameFromResourcePath($resourcePath);
        $resourceName = $this->getResourceNameFromResourcePath($resourcePath);

        // Symfony 4.0+ always returns a single bundle
        $bundles = $this->kernel->getBundle($bundleName, false);

        // So we need to hack it to support both Symfony 3.4 and Symfony 4.0+
        if (!is_array($bundles)) {
            $bundles = [$bundles];
        }

        foreach ($bundles as $bundle) {
            $path = sprintf('%s/%s/%s', $theme->getPath(), $bundle->getName(), $resourceName);

            if ($this->filesystem->exists($path)) {
                return $path;
            }
        }

        throw new ResourceNotFoundException($resourcePath, $theme);
    }

    private function assertResourcePathIsValid(string $resourcePath, ThemeInterface $theme): void
    {
        if (0 !== strpos($resourcePath, '@')) {
            throw new \InvalidArgumentException(sprintf('Bundle resource path (given "%s") should start with an "@".', $resourcePath));
        }

        if (false !== strpos($resourcePath, '..')) {
            throw new \InvalidArgumentException(sprintf('File name "%s" contains invalid characters (..).', $resourcePath));
        }

        if (false !== strpos($resourcePath, 'Resources/')) {
            throw new ResourceNotFoundException($resourcePath, $theme);
        }
    }

    /**
     * @param string $resourcePath
     *
     * @return string
     */
    private function getBundleNameFromResourcePath(string $resourcePath): string
    {
        return substr($resourcePath, 1, strpos($resourcePath, '/') - 1).'Bundle';
    }

    /**
     * @param string $resourcePath
     *
     * @return string
     */
    private function getResourceNameFromResourcePath(string $resourcePath): string
    {
        return substr($resourcePath, strpos($resourcePath, '/') +  1);
    }
}

maybe we should handle the two structure in this class ?

IMO it's prematured to recommand to change the new structure #9729 (comment)
We need to have more real app testing.

@CoderMaggie CoderMaggie added Documentation Documentation related issues and PRs - requests, fixes, proposals. DX Issues and PRs aimed at improving Developer eXperience. labels Oct 2, 2018
@pamil
Copy link
Contributor

pamil commented Oct 11, 2018

#9804 will fix this issue.

@pamil pamil closed this as completed Oct 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Documentation related issues and PRs - requests, fixes, proposals. DX Issues and PRs aimed at improving Developer eXperience.
Projects
None yet
Development

No branches or pull requests

3 participants