sonata-project / SonataPageBundle

This bundle provides a Site and Page management through container and block services
https://docs.sonata-project.org/projects/SonataPageBundle
MIT License
218 stars 209 forks source link

Add a runtime for the symfony/runtime component. #1727

Closed GeraudBourdin closed 11 months ago

GeraudBourdin commented 11 months ago

Symfony runtime for SonataPage

Add a SonataPage runtime to avoid custom configuration in the public/index.php

use the configuration above for testing this.

composer require symfony/runtime

public/index.php :


    use App\Kernel;

    require_once dirname(__DIR__).'/vendor/autoload_runtime.php';

    return function (array $context) {
        return new Kernel($context['APP_ENV'], (bool) $context['APP_DEBUG']);
    };

composer.json :

{
    // ... other composer configs ...
    "extra": {
        // ... other composer configs ...
        "runtime": {
            "class": "Sonata\\PageBundle\\Runtime\\SonataPageRuntime",
            "multisite": "host_with_path_by_locale"
        }
    }
}

multisite is not manatory, by default host_with_path_by_locale is selected

Needs to refresh the autoload file: composer dump-autoload

Documentation used : https://symfony.com/doc/current/components/runtime.html

Closes #1604

Changelog

### Added
- Support for the symfony runtime component
VincentLanglet commented 11 months ago

ci is failing

GeraudBourdin commented 11 months ago

Can you change isRequired to ->defaultValue('host') in the Config too ?

done

VincentLanglet commented 11 months ago

Can you change isRequired to ->defaultValue('host') in the Config too ?

done

Thanks.

To me, code is ready. But now we need to update the doc.

In https://github.com/sonata-project/SonataPageBundle/blob/c0b2c445d423b8880ca53be8ce20442a6d29c2e8/docs/reference/advanced_configuration.rst#L29 the part multisite: ~ # Required need to be removed (and maybe in some other files).

This page need to be update too https://github.com/sonata-project/SonataPageBundle/blob/c0b2c445d423b8880ca53be8ce20442a6d29c2e8/docs/reference/multisite.rst#L2

GeraudBourdin commented 11 months ago

Can you change isRequired to ->defaultValue('host') in the Config too ?

done

Thanks.

To me, code is ready. But now we need to update the doc.

In

https://github.com/sonata-project/SonataPageBundle/blob/c0b2c445d423b8880ca53be8ce20442a6d29c2e8/docs/reference/advanced_configuration.rst#L29

the part multisite: ~ # Required need to be removed (and maybe in some other files). This page need to be update too

https://github.com/sonata-project/SonataPageBundle/blob/c0b2c445d423b8880ca53be8ce20442a6d29c2e8/docs/reference/multisite.rst#L2

Doc updated. Hope that's ok.

VincentLanglet commented 11 months ago
multisite: string,

need to be changed in

multisite?: string,

and Ci need to be fixed https://github.com/sonata-project/SonataPageBundle/actions/runs/7003266717/job/19048641209?pr=1727

VincentLanglet commented 11 months ago

Thanks for your time and your work.

I'll wait one more day if someone has something to say and I'll merge it ^^

eerison commented 11 months ago

Thanks for your time and your work.

I'll wait one more day if someone has something to say and I'll merge it ^^

should be good confirm that, it is working ;)

eerison commented 11 months ago

For the deprecation in the tests, just add

* @group legacy
*
* NEXT_MAJOR: Remove this class

at the top of the RequestFactoryTest class

@GeraudBourdin ☝🏼

GeraudBourdin commented 11 months ago

ok i missed one line between annotation and NEXT_major.

VincentLanglet commented 11 months ago

ok i missed one line between annotation and NEXT_major.

/**
 * @group legacy
 *
 * NEXT_MAJOR: Remove this class

was for the RequestFactoryTest and not the runtime one ;)

eerison commented 11 months ago

ok i missed one line between annotation and NEXT_major.

you are also missing this comment 😄

https://github.com/sonata-project/SonataPageBundle/pull/1727#pullrequestreview-1754787850

eerison commented 11 months ago

ok i missed one line between annotation and NEXT_major.

/**
 * @group legacy
 *
 * NEXT_MAJOR: Remove this class

was for the RequestFactoryTest and not the runtime one ;)

I guess we need to do something else to psalm didn't complain about deprecated method. Do you have any idea what should we do in this case @VincentLanglet ?

eerison commented 11 months ago

Looks good for me 💪

Thank you @GeraudBourdin ,it is a really nice feature ♥️

Good job 👏👏

tdumalin commented 11 months ago

Hi everyone,

How can I use the multisite: host_with_path before the next release ? For now if i tried to use this option I got the following error:

You must change the main Request object in the front controller (app.php) in order to use the `host_with_path` strategy

I guess there is some workaround to do in the public/index.php ?

Thanks for the help

VincentLanglet commented 11 months ago

I just made a new release

tdumalin commented 11 months ago

@VincentLanglet, perfect thank you very much ! Let's use this new feature :+1:

haivala commented 11 months ago

I installed this but now I cannot access web_profiler in dev anymore. Anyone experiencing the same? going to _profiler/{hash} url is giving me An exception has been thrown during the rendering of a template ("You must change the main Request object in the front controller (app.php) in order to use thehost_with_path_by_localestrategy").

eerison commented 11 months ago

I installed this but now I cannot access web_profiler in dev anymore. Anyone experiencing the same? going to _profiler/{hash} url is giving me An exception has been thrown during the rendering of a template ("You must change the main Request object in the front controller (app.php) in order to use thehost_with_path_by_localestrategy").

did you add the runtime?

eerison commented 11 months ago

@haivala

did you do this: https://docs.sonata-project.org/projects/SonataPageBundle/en/4.x/reference/multisite/#host-and-path-strategy ?

eerison commented 11 months ago

@haivala

did you do this: https://docs.sonata-project.org/projects/SonataPageBundle/en/4.x/reference/multisite/#host-and-path-strategy ?

in theory you didn't add the runtime: https://github.com/sonata-project/SonataPageBundle/blob/b009becd106bab26d572bf81141b7d398d3fa81b/src/Site/HostPathByLocaleSiteSelector.php#L29 and run composer dump-autoload.

Note: I guess we could update this message as well :D

VincentLanglet commented 11 months ago

@haivala did you do this: docs.sonata-project.org/projects/SonataPageBundle/en/4.x/reference/multisite/#host-and-path-strategy ?

in theory you didn't add the runtime:

https://github.com/sonata-project/SonataPageBundle/blob/b009becd106bab26d572bf81141b7d398d3fa81b/src/Site/HostPathByLocaleSiteSelector.php#L29

and run composer dump-autoload. Note: I guess we could update this message as well :D

PR welcomed for the message if it's the reason of the issue ;)

eerison commented 11 months ago

@haivala did you do this: docs.sonata-project.org/projects/SonataPageBundle/en/4.x/reference/multisite/#host-and-path-strategy ?

in theory you didn't add the runtime: https://github.com/sonata-project/SonataPageBundle/blob/b009becd106bab26d572bf81141b7d398d3fa81b/src/Site/HostPathByLocaleSiteSelector.php#L29

and run composer dump-autoload. Note: I guess we could update this message as well :D

PR welcomed for the message if it's the reason of the issue ;)

https://github.com/sonata-project/SonataPageBundle/pull/1737

haivala commented 11 months ago

Yes. I did. Runtime was added automatically with the pagebundle update. Everything else is working as before. Just cannot access the web profiler anymore.

haivala commented 11 months ago

Feels like ignore_route_patterns config option is not working anymore.

eerison commented 11 months ago

Feels like ignore_route_patterns config option is not working anymore.

Please open an issue with