Symfony 5 compatibility and Switch to SyliusThemeBundle #19

mario-fehr · 2020-08-26T05:37:13Z

Q	A
Bug fix?	no
New feature?	no
BC breaks?	yes
Deprecations?	no
Fixed tickets	fixes #17
Related issues/PRs	#17
License	MIT

What's in this PR?

This PR removes LiipThemeBundle dependency and added SyliusThemeBundle to have Symfony 5 compatibility.

BC Breaks/Deprecations

Due to the exchange of bundles, the folder structure must be adapted and the configuration exchanged for old projects.

To Do

Update Tests
Update Upgrade.md
Update Dokumentation

mario-fehr · 2020-08-26T06:00:30Z

@danrot @alexander-schranz Alex told me to open a PR to show my work so far testing the SyliusThemeBundle in combination with Sulu. But I need some help with the test and pipline.

danrot · 2020-08-28T06:55:16Z

@mfehr94 But otherwise it is already working as expected? If it is only about the tests you can get back to me some day in the office, that's probably the easiest way to fix that 🙂

JKetelaar · 2020-09-23T14:09:28Z

We're using this in our development environment. Seems to work perfectly!

For anyone who wants to have a workaround for Symfony 5 & Theme support:
composer.json

    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/mfehr94/SuluThemeBundle"
        }
    ]

...

    "require": {
        "sulu/theme-bundle": "dev-feature/symfony5"
    }

JKetelaar · 2020-09-24T12:36:18Z

After testing this, it does cause issues when editing a page from the Sulu admin. I will provide a stacktrace later today.

dsb-hannes-flaig · 2020-10-29T09:54:20Z

Ive followed @JKetelaar approach and added the Bundle to a fresh Sulu skeleton made with composer create-project sulu/skeleton cms_sulu_skeleton -n.

Problem
When trying to save a page in the admin panel, the preview area does not show a preview but instead displays a symfony error:

Stacktrace:

TypeError:
Argument 1 passed to Sylius\Bundle\ThemeBundle\Repository\InMemoryThemeRepository::findOneByName() must be of the type string, null given, called in /var/www/fly/cms_sulu_skeleton/vendor/sulu/theme-bundle/EventListener/SetThemeEventListener.php on line 63

  at vendor/sylius/theme-bundle/src/Repository/InMemoryThemeRepository.php:42
  at Sylius\Bundle\ThemeBundle\Repository\InMemoryThemeRepository->findOneByName(null)
     (vendor/sulu/theme-bundle/EventListener/SetThemeEventListener.php:63)
  at Sulu\Bundle\ThemeBundle\EventListener\SetThemeEventListener->setActiveThemeOnPreviewPreRender(object(PreRenderEvent), 'sulu.preview.pre-render', object(EventDispatcher))
     (vendor/symfony/event-dispatcher/EventDispatcher.php:270)
  at Symfony\Component\EventDispatcher\EventDispatcher::Symfony\Component\EventDispatcher\{closure}(object(PreRenderEvent), 'sulu.preview.pre-render', object(EventDispatcher))
     (vendor/symfony/event-dispatcher/EventDispatcher.php:230)
  at Symfony\Component\EventDispatcher\EventDispatcher->callListeners(array(object(Closure)), 'sulu.preview.pre-render', object(PreRenderEvent))
     (vendor/symfony/event-dispatcher/EventDispatcher.php:59)
  at Symfony\Component\EventDispatcher\EventDispatcher->dispatch(object(PreRenderEvent), 'sulu.preview.pre-render')
     (vendor/sulu/sulu/src/Sulu/Bundle/PreviewBundle/Preview/Renderer/PreviewRenderer.php:173)
  at Sulu\Bundle\PreviewBundle\Preview\Renderer\PreviewRenderer->render(object(PageDocument), '10334765-80cb-4cb1-aec5-dac045963da6', 'example', 'en', false, -1)
     (vendor/sulu/sulu/src/Sulu/Bundle/PreviewBundle/Preview/Preview.php:140)
  at Sulu\Bundle\PreviewBundle\Preview\Preview->render('2c3502e7ea458d8cd27fa865fa651f72', 'example', 'en', -1)
     (vendor/sulu/sulu/src/Sulu/Bundle/PreviewBundle/Controller/PreviewController.php:77)
  at Sulu\Bundle\PreviewBundle\Controller\PreviewController->renderAction(object(Request))
     (vendor/symfony/http-kernel/HttpKernel.php:157)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor/symfony/http-kernel/HttpKernel.php:79)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor/symfony/http-kernel/Kernel.php:196)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (public/index.php:66)

alexander-schranz · 2020-10-29T10:06:02Z

@SirCoffeeMonkey Thank you for report. Did you configure the <theme> in your webspace.xml? Can you debug what $event->getAttribute('webspace') does return?

mario-fehr · 2020-10-29T11:26:14Z

@SirCoffeeMonkey if you have <theme> configured, you need to make shure that you match the naming convention of the SyliusThemeBundle. https://github.com/Sylius/SyliusThemeBundle/blob/master/docs/theme_configuration_reference.md
So if we take this example it must look like this <theme>vendor/sylius-theme</theme> Unfortiantly the naming convention for the theme name must be composer style with a slash for example vendor/sylius-theme or massive/crimson-theme

mario-fehr · 2020-10-29T11:31:01Z

Addition to that your theme need a composer.json (it is possible to change the file name by config) file with the theme infos https://github.com/Sylius/SyliusThemeBundle/blob/master/docs/index.md.

My sylius_theme.yaml looks like this:

    sources:
        filesystem:
            filename: theme.json // default: composer.json
            directories:
                - "%kernel.project_dir%/themes"

and the theme.json file in my themes/CrimsonTheme folder like this:

  "name": "massive/crimson-theme",
  "authors": [
    {
      "name": "Mario Fehr",
    }
  ],
  "extra": {
    "sylius-theme": {
      "title": "Crimson Theme"
    }
  }
}

dsb-hannes-flaig · 2020-10-29T11:33:41Z

@alexander-schranz thank you for your swift response. I forgot to properly configure the sylius theme bundle. i have added following line to my webspace.xml:
<theme>test/test-theme</theme>
Then i added the following lines to the config/packages/sylius_theme.yaml (which you have to manually create):

sylius_theme:
  sources:
    filesystem:
      directories:
        - "%kernel.project_dir%/themes"

Finally i created following folder structure:
<ProjectDir>/themes/YourTheme
and added a composer.json in the YourTheme directory, with the following content:

{
  "name" : "test/test-theme",
  "extra": {
    "sylius-theme": {
      "title": "Test Theme"
    }
  }
}

For more details as to what can be configured in the theme composer.json you can refer to the links @mfehr94 kindly provided.

Now it is running and no longer throwing any errors 👍 😁
Thank you for putting me on the right path.

alexander-schranz

@Prokyonn Some markdown syntax change requests.

UPGRADE.md

alexander-schranz · 2020-11-12T11:00:57Z

UPGRADE.md

@@ -0,0 +1,105 @@
+# Upgrade
+
+## 2.1


This will be a new major release as 3.0

alexander-schranz

Would focus the UPGRADE on a specific theme example.

alexander-schranz · 2020-11-12T12:51:56Z

phpunit.xml.dist

@@ -16,4 +16,14 @@
            </exclude>
        </whitelist>
    </filter>
+
+    <php>
+        <env name="SYMFONY_PHPUNIT_VERSION" value="8.5.9"/>


as using the phpunit bridge 8 is enough that the newest phpunit bridge is always installed. ^8.5.9 would only be required when we require phpunit package in the composer.json.

Suggested change

<env name="SYMFONY_PHPUNIT_VERSION" value="8.5.9"/>

<env name="SYMFONY_PHPUNIT_VERSION" value="8.5.9"/>

alexander-schranz · 2020-11-12T12:54:46Z

UPGRADE.md

+composer remove liip/theme-bundle --no-update
+
+# Install new theme-bundle
+composer require sylius/theme-bundle


We should also add sulu/theme-bundle update here:

Suggested change

composer require sylius/theme-bundle

composer require sylius/theme-bundle:"^2.0" --no-update

composer require sulu/theme-bundle:"^3.0" --with-dependencies

alexander-schranz · 2020-11-12T13:17:35Z

UPGRADE.md

+
+## 3.0
+
+### Switch from LiipThemeBundle to SyliusThemeBundle


@lsmith77 Parts of this UPGRADE.md could be used in the LiipThemeBundle docs when the project will be marked as abandoned.

alexander-schranz · 2020-11-12T13:19:11Z

UPGRADE.md

+├── bin
+├── config
+├── ...
+├── templates


would directly use themes folder here like its in the sylius documentation

mario-fehr · 2020-11-12T13:36:12Z

@Prokyonn thanks for fixing the tests and dokumentationen

README.md

niklasnatter · 2020-11-12T14:28:02Z

README.md

+
+### Configure your themes
+
+Every theme must have its own configuration file in form of a `composer.json`.


should we use composer.json or theme.json in the examples?

UPGRADE.md

Co-authored-by: nnatter <niklas.natter@gmail.com>

README.md

alexander-schranz · 2020-11-19T10:10:17Z

Thank you @mfehr94 @Prokyonn !

mario-fehr and others added 5 commits August 21, 2020 18:58

Update composer.json

43b56e9

Update composer.json

253065d

switch from LiipThemeBundle to SyliusThemeBundle

2ab31c4

Try to fix tests.

b6af8ae

remove final bypass.

bdd85b8

mario-fehr changed the title ~~Feature/symfony5~~ Symfony 5 compatibility and Switch to SyliusThemeBundle Aug 26, 2020

fix pipline

187ccf8

Cleanup and fix tests

4238a72

Prokyonn force-pushed the feature/symfony5 branch from 1a1156d to 4238a72 Compare November 11, 2020 15:04

Add phpunit-bridge

6f48444

Prokyonn force-pushed the feature/symfony5 branch from 307348e to 6f48444 Compare November 11, 2020 15:42

Add UPGRADE.md file

e3b8e9a

alexander-schranz requested changes Nov 12, 2020

View reviewed changes

UPGRADE.md Outdated Show resolved Hide resolved

UPGRADE.md Outdated Show resolved Hide resolved

UPGRADE.md Outdated Show resolved Hide resolved

UPGRADE.md Outdated Show resolved Hide resolved

Add empty lines after headlines

9253c8f

Prokyonn marked this pull request as ready for review November 12, 2020 09:25

Prokyonn mentioned this pull request Nov 12, 2020

Adjust theming documentation sulu/sulu-docs#582

Merged

alexander-schranz reviewed Nov 12, 2020

View reviewed changes

UPGRADE.md Outdated

@@ -0,0 +1,105 @@

# Upgrade

## 2.1

Copy link

Member

alexander-schranz Nov 12, 2020

Choose a reason for hiding this comment

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

This will be a new major release as 3.0

alexander-schranz requested changes Nov 12, 2020

View reviewed changes

Prokyonn force-pushed the feature/symfony5 branch from f468652 to 36684e6 Compare November 12, 2020 12:49

alexander-schranz reviewed Nov 12, 2020

View reviewed changes

Prokyonn force-pushed the feature/symfony5 branch from 36684e6 to 3abd106 Compare November 12, 2020 12:59

Update README.md

4526b1f

Prokyonn force-pushed the feature/symfony5 branch from 3abd106 to 4526b1f Compare November 12, 2020 13:11

alexander-schranz reviewed Nov 12, 2020

View reviewed changes

Update template path in readme and update.md

32f01d2

niklasnatter reviewed Nov 12, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

niklasnatter reviewed Nov 12, 2020

View reviewed changes

README.md Outdated Show resolved Hide resolved

niklasnatter reviewed Nov 12, 2020

View reviewed changes

UPGRADE.md Outdated Show resolved Hide resolved

Prokyonn and others added 2 commits November 12, 2020 15:33

Apply suggestions from code review

6a99b8e

Co-authored-by: nnatter <niklas.natter@gmail.com>

Fix theme configuration file name

b9bac68

alexander-schranz approved these changes Nov 19, 2020

View reviewed changes

README.md Show resolved Hide resolved

Update README.md

1049812

alexander-schranz added the enhancement label Nov 19, 2020

alexander-schranz merged commit 5a926a9 into sulu:master Nov 19, 2020

mario-fehr deleted the feature/symfony5 branch November 19, 2020 10:44

rogamoore mentioned this pull request Feb 13, 2023

Remove reference to LiipThemeBundle and dispatch event instead sulu/sulu#7004

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Symfony 5 compatibility and Switch to SyliusThemeBundle #19

Symfony 5 compatibility and Switch to SyliusThemeBundle #19

mario-fehr commented Aug 26, 2020 •

edited by Prokyonn

Loading

mario-fehr commented Aug 26, 2020

danrot commented Aug 28, 2020

JKetelaar commented Sep 23, 2020 •

edited

Loading

JKetelaar commented Sep 24, 2020

dsb-hannes-flaig commented Oct 29, 2020 •

edited

Loading

alexander-schranz commented Oct 29, 2020

mario-fehr commented Oct 29, 2020

mario-fehr commented Oct 29, 2020 •

edited

Loading

dsb-hannes-flaig commented Oct 29, 2020 •

edited

Loading

alexander-schranz left a comment

alexander-schranz Nov 12, 2020

alexander-schranz left a comment

alexander-schranz Nov 12, 2020 •

edited

Loading

alexander-schranz Nov 12, 2020 •

edited

Loading

alexander-schranz Nov 12, 2020 •

edited

Loading

alexander-schranz Nov 12, 2020

mario-fehr commented Nov 12, 2020

niklasnatter Nov 12, 2020

alexander-schranz commented Nov 19, 2020

	<env name="SYMFONY_PHPUNIT_VERSION" value="8.5.9"/>
	<env name="SYMFONY_PHPUNIT_VERSION" value="8.5.9"/>

	composer require sylius/theme-bundle
	composer require sylius/theme-bundle:"^2.0" --no-update
	composer require sulu/theme-bundle:"^3.0" --with-dependencies


		### Configure your themes

		Every theme must have its own configuration file in form of a `composer.json`.

Symfony 5 compatibility and Switch to SyliusThemeBundle #19

Symfony 5 compatibility and Switch to SyliusThemeBundle #19

Conversation

mario-fehr commented Aug 26, 2020 • edited by Prokyonn Loading

What's in this PR?

BC Breaks/Deprecations

To Do

mario-fehr commented Aug 26, 2020

danrot commented Aug 28, 2020

JKetelaar commented Sep 23, 2020 • edited Loading

JKetelaar commented Sep 24, 2020

dsb-hannes-flaig commented Oct 29, 2020 • edited Loading

alexander-schranz commented Oct 29, 2020

mario-fehr commented Oct 29, 2020

mario-fehr commented Oct 29, 2020 • edited Loading

dsb-hannes-flaig commented Oct 29, 2020 • edited Loading

alexander-schranz left a comment

Choose a reason for hiding this comment

alexander-schranz Nov 12, 2020

Choose a reason for hiding this comment

alexander-schranz left a comment

Choose a reason for hiding this comment

alexander-schranz Nov 12, 2020 • edited Loading

Choose a reason for hiding this comment

alexander-schranz Nov 12, 2020 • edited Loading

Choose a reason for hiding this comment

alexander-schranz Nov 12, 2020 • edited Loading

Choose a reason for hiding this comment

alexander-schranz Nov 12, 2020

Choose a reason for hiding this comment

mario-fehr commented Nov 12, 2020

niklasnatter Nov 12, 2020

Choose a reason for hiding this comment

alexander-schranz commented Nov 19, 2020

mario-fehr commented Aug 26, 2020 •

edited by Prokyonn

Loading

JKetelaar commented Sep 23, 2020 •

edited

Loading

dsb-hannes-flaig commented Oct 29, 2020 •

edited

Loading

mario-fehr commented Oct 29, 2020 •

edited

Loading

dsb-hannes-flaig commented Oct 29, 2020 •

edited

Loading

alexander-schranz Nov 12, 2020 •

edited

Loading

alexander-schranz Nov 12, 2020 •

edited

Loading

alexander-schranz Nov 12, 2020 •

edited

Loading