search

sulu / SuluArticleBundle

Bundle for managing localized content-rich entities like blog-posts in the Sulu content management system
MIT License
52 stars 77 forks source link

Fix types filter with website user #626

Closed luca-rath closed 1 year ago

luca-rath commented 1 year ago
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets -
Related issues/PRs https://github.com/tommysonsylverstone/sulu-elasticsearch-user-bug
License MIT

What's in this PR?

This PR fixes the problem described in https://github.com/tommysonsylverstone/sulu-elasticsearch-user-bug

Why?

The ArticleDataProvider::getTypes() method is used to call ConfigurationBuilder::enableTypes() (see https://github.com/sulu/SuluArticleBundle/blob/2.x/Content/ArticleDataProvider.php#L499). This method only returns types, if there is a TokenStorage with a user, to use the user's locale to return the title of the templates in the correct locale. This works fine, because in most cases, this just returns the types in the admin context and not in the website context. But as soon as there is a website user, this method also returns types, and the ArticleDataProvider doesn't expect that, because those types are used in the ContentType::getDefaultParams(). Because that logic is in sulu itself, we should not touch it and instead work around that here in the article bundle.

alexander-schranz commented 1 year ago

The strange thing is that the logic here:

https://github.com/sulu/SuluArticleBundle/blob/af36456d4517f3dd0fb3dfa88804245d56081e0a/Content/ArticleDataProvider.php#L506

And here:

https://github.com/sulu/sulu/blob/08863981d9e607dd1432db68eec2dac710732f74/src/Sulu/Component/Content/SmartContent/PageDataProvider.php#L464

And here:

https://github.com/sulu/sulu/blob/08863981d9e607dd1432db68eec2dac710732f74/src/Sulu/Bundle/SnippetBundle/Content/SnippetDataProvider.php#L267

Are pages and snippet behaving correctly and don't have the same issue with any types filters?

tommysonsylverstone commented 1 year ago

Hello, any news about this? I tested the fix directly in the vendors and it works well. Thank you in advance for the update!

alexander-schranz commented 1 year ago

I'm merging this also when I think there some changes maybe also in the core then required.

sevenGroupFrance commented 1 year ago

Hi team,

Any chance to fix this in sulu:2.4 ? Thank you for your help.