[Theme] Support for settings

[pamil]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	yes (Twig functions: removed sylius_settings_get and sylius_settings_has, renamed sylius_settings_all to sylius_settings)
Deprecations?	no
Related tickets	-
License	MIT

[pjedrzejewski]

Awesome work, could we add notes to UPGRADE file about settings changes? I think it would be useful for people upgrading. /cc @steffenbrem

[pamil]

@pjedrzejewski here it goes 🌵

[koemeet]

@pamil Isn't it a better way to ditch the ThemeSettingsManager etc. and it's custom resolver interface and use the SettingsResolverInterface? My PR with those changes was made for things like theme settings. This is how I would implement it:

Have a ThemeSettingsResolver/CurrentThemeSettingsResolver class that implements the SettingsResolverInterface. In there, it would inject the sylius.context.theme service and the sylius.repository.setting service. With that you can retrieve the settings that are linked to that theme. The only changes left are that we would need a one-to-many unidirectional relationship from a Theme to Settings (http://doctrine-orm.readthedocs.org/projects/doctrine-orm/en/latest/reference/association-mapping.html#one-to-many-unidirectional-with-join-table).

If you build it that way, you have a much more powerful setup and you could easily implement things like saving theme settings as draft, as in versioning the themes settings.

I am using the settings bundle for themes myself and the above solution works wonderful. I think it is worth considering if you also want that approach in Sylius. The current approach limits the power of theme settings I think.

[koemeet]

You would also need a one-to-one from the Theme to Settings for the current active theme settings. The one-to-many is optional, but nice to have if you want to do things like saving settings as a draft for that theme.

[pamil]

Having a relationship on a theme wouldn't be a solution right now as I've just opened #4897 which deletes the filesystem -> database synchronization, which has caused a lot of confusion and weird technical decision like FailsafeThemeRepository catching every exception thrown by Doctrine theme repository and using different sources for getting themes.

I may miss something in the approach you proposed, does it mean that there will be a schemaAlias theme and different namespaces for each theme or different schemas aliases for each theme? I would rather want to keep all the stuff as less coupled to currently used theme as possible.

[koemeet]

@pamil Ah okay, yeah if you don't have a theme reference in the database it does not make much sense to do it the way I described. But for what it's worth, you would indeed have a theme schema alias, which would be build dynamically. There would be no namespace, since the ThemeSettingsResolver only needs the current theme and the theme alias is for resolving to that specific resolver.

Settings would not be coupled to themes, since you would just have an activeSettings property on a Theme. That way you can also do cascading etc.

[pamil]

Anyway, do you know what are the example use-cases for settings namespaces?

[koemeet]

Personally I would only use namespaces when I want to simply store multiple different settings somewhere, but can't link the settings to an existing entity. I never hit that issue, can't think of an use case that shares the same schema and needs to be different (having multiple "instances") without being related to an entity.

[pamil]

Thanks! 🎉 At first I thought it might be wise to have namespace theme_settings and theme codes as schemas aliases, but then I had realised that the schemas and resolvers are get only by the schema alias so that one could possibly want to have the same schema alias in different namespace and it might cause some conflicts :)

[pjedrzejewski]

Nice work Kamil, thank you!