Skip to content

Drop older doctrine persistence, allow collections 2.0, update psalm, incresing it to level 2 and avoid deprecations#889

Merged
VincentLanglet merged 6 commits intosonata-project:4.xfrom
jordisala1991:hotfix/update
Mar 5, 2023
Merged

Drop older doctrine persistence, allow collections 2.0, update psalm, incresing it to level 2 and avoid deprecations#889
VincentLanglet merged 6 commits intosonata-project:4.xfrom
jordisala1991:hotfix/update

Conversation

@jordisala1991
Copy link
Copy Markdown
Member

@jordisala1991 jordisala1991 commented Mar 4, 2023

Subject

I am targeting this branch, because this should be BC and probably fix at least one bug.

Changelog

### Added
- Added support for `doctrine/collections` ^2.0.

### Removed
- Removed support for `doctrine/persistence` ^2.0.

### Fixed
- Avoid deprecations for SonataAdminBundle filters.

/**
* @return array<array-key, string>
*
* @phpstan-param Options<array{
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Do you know if this can be done better @VincentLanglet ?

PHPStan understand the options param has 2 keys, but it is unable to know that 'context' will be ContextInterface or null. It is mixing keys and values.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think I saw it, the issue opened by you: phpstan/phpstan-symfony#322

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We might supress the issues raised, and wait for a fix on phpstan side.

Comment thread src/Entity/CategoryManager.php Outdated
@jordisala1991 jordisala1991 marked this pull request as ready for review March 4, 2023 18:39
@jordisala1991 jordisala1991 requested review from a team and VincentLanglet March 4, 2023 18:39

$parent = $category->getParent();
if (null !== $parent) {
$parent->addChild($category);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This should have been removed when we deprecated/removed the disableLazyChildren collection here: #765


if (null !== $contextId) {
$queryBuilder->andWhere('c.context = :context')->setParameter('context', $contextId, Types::OBJECT);
$queryBuilder->andWhere('c.context = :context')->setParameter('context', $contextId);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This was failing when trying to query with a non mocked CategoryManager on CategoryManagerTest, wdyt? ContextId is not an object, but a string.


$parent = $category->getParent();
if (null !== $parent) {
$parent->addChild($category);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

same as the other lazy comment.

@@ -1,10 +0,0 @@
<?xml version="1.0" encoding="utf-8"?>
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Those skeletons were used by the EasyExtends bundle, should have been removed by 4.0

], $fieldOptions);

return $formMapper->create($formField, ModelListType::class, $fieldOptions);
return $formMapper->getFormBuilder()->create($formField, ModelListType::class, $fieldOptions);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Here I am trying to avoid the deprecated call to ->create(). Following same strategy as we did for blocks on PageBundle: https://github.com/sonata-project/SonataPageBundle/blob/4.x/src/Block/SharedBlockBlockService.php#L148

There is not an easy way to test if the whole block works or not, but I think I will leave it for a follow up PR. My guess is that those blocks does not work, with or without this PR.

@jordisala1991 jordisala1991 changed the title Update deps Drop older doctrine persistence, allow collections 2.0, update psalm, incresing it to level 2 and avoid deprecations Mar 4, 2023
@VincentLanglet VincentLanglet merged commit 5b37d65 into sonata-project:4.x Mar 5, 2023
@jordisala1991 jordisala1991 deleted the hotfix/update branch March 5, 2023 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants