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

Using Symfony configuration for webspaces #7218

Open
wants to merge 57 commits into
base: 3.0
Choose a base branch
from

Conversation

mamazu
Copy link
Contributor

@mamazu mamazu commented Dec 1, 2023

Q A
Bug fix? no
New feature? kinda
BC breaks? yes
Deprecations? no
Fixed tickets -
Related issues/PRs -
License MIT
Documentation PR sulu/sulu-docs#790

What's in this PR?

Creating a way to configure webspaces with Symfony configuration.

Why?

Currently Sulu has it's own XML parser for webspace configs. This is a lot of maintenance and hard to extend. Using the Symfony config solves both

Example Usage

Just write your configs like before. But now you can also use YAML and PHP.

To Do

  • Create a documentation PR

  • Make sublocales possible in the config (at most two)

  • Document this ^

  • Document how to extend the configuration

  • It should work with the same configuration as before

  • Making the WebspaceCollection read only

  • Checking all Exceptions have been turned into validation

    • ExpectedDefaultTemplatesNotFound.php (moved to WebspaceConfiguration)
    • InvalidAmountOfDefaultErrorTemplateException.php (still there)
    • InvalidCustomUrlException.php (moved to WebspaceConfiguration)
    • InvalidDefaultErrorTemplateException.php (moved to WebspaceConfiguration)
    • InvalidDefaultLocalizationException.php (moved to WebspaceConfiguration)
    • InvalidErrorTemplateException.php (moved to WebspaceConfiguration)
    • InvalidPortalDefaultLocalizationException.php (moved to WebspaceConfiguration)
    • InvalidUrlDefinitionException.php (still exists)
    • InvalidWebspaceDefaultLocalizationException.php (moved to WebspaceConfiguration)
    • InvalidWebspaceDefaultSegmentException.php (moved to WebspaceConfiguration)
    • PortalDefaultLocalizationNotFoundException.php (still exists)
    • WebspaceDefaultLocalizationNotFoundException.php (moved to WebspaceConfiguration)
    • WebspaceDefaultSegmentNotFoundException.php (moved to WebspaceConfiguration)
    • WebspaceException.php (base exception is still there)

@mamazu mamazu changed the title Using Symfony configuration for webspaces Draft: Using Symfony configuration for webspaces Dec 1, 2023
@mamazu mamazu marked this pull request as draft December 1, 2023 10:34
->addDefaultsIfNotSet()
->children()
->scalarNode('config_dir')
->defaultValue('%kernel.project_dir%/config/webspaces')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add this directory to the kernel maybe to also load webspaces from there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we can do that in Sulu code?

config/packages/dev/webspaces.xml Outdated Show resolved Hide resolved
@mamazu mamazu force-pushed the using_symonfy_for_webspaces branch from 3722ee9 to dbd4952 Compare December 3, 2023 23:32
src/Sulu/Component/Webspace/Manager/WebspaceManager.php Outdated Show resolved Hide resolved
@@ -36,7 +40,7 @@
</navigation>

<resource-locator>
<strategy>short</strategy>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that a real strategy? If so, then I couldn't find any classes implementing the strategy.

@mamazu mamazu self-assigned this Dec 17, 2023
@mamazu mamazu force-pushed the using_symonfy_for_webspaces branch from e09f570 to bf67a98 Compare March 9, 2024 16:59
@@ -17,7 +17,7 @@ interface SystemStoreInterface
{
public function getSystem(): ?string;

public function setSystem(string $system): void;
public function setSystem(?string $system): void;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

BC break

@mamazu mamazu force-pushed the using_symonfy_for_webspaces branch 4 times, most recently from 8dd5614 to 1793fd1 Compare March 9, 2024 19:19
@mamazu mamazu force-pushed the using_symonfy_for_webspaces branch from 1793fd1 to fbb3645 Compare March 10, 2024 13:47
@mamazu mamazu marked this pull request as ready for review March 13, 2024 14:25
@mamazu
Copy link
Contributor Author

mamazu commented Mar 13, 2024

Migration guide

  1. Make sure that all webspace configuration files are under config/webspaces/ directory.
  2. Select the starting webspace-tag in each file
<webspace xmlns="http://schemas.sulu.io/webspace/webspace"
          xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          xsi:schemaLocation="http://schemas.sulu.io/webspace/webspace http://schemas.sulu.io/webspace/webspace-1.1.xsd">

and replace it with this:

<services:container xmlns="http://example.org/schema/dic/sulu_website"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns:services="http://symfony.com/schema/dic/services"
    xsi:schemaLocation="http://symfony.com/schema/dic/services
        https://symfony.com/schema/dic/services/services-1.0.xsd"
    >
<config>
<webspace>
  1. And at the end of each file after the closing webspace tag add those lines:
</config>
</services:container>

@mamazu mamazu marked this pull request as draft March 13, 2024 14:31
@mamazu mamazu changed the title Draft: Using Symfony configuration for webspaces /Using Symfony configuration for webspaces Mar 13, 2024
@mamazu mamazu changed the title /Using Symfony configuration for webspaces Using Symfony configuration for webspaces Mar 13, 2024
@mamazu mamazu marked this pull request as ready for review May 18, 2024 12:21
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 this pull request may close these issues.

None yet

2 participants