sonata-project / SonataAdminBundle

The missing Symfony Admin Generator
https://docs.sonata-project.org/projects/SonataAdminBundle
MIT License
2.11k stars 1.26k forks source link

Remove assert of Exporter instance in CRUDController #7992

Closed pkameisha closed 1 year ago

pkameisha commented 1 year ago

Feature Request

I have an issue in project while upgrading to the symfony 5.4 and latest sonata packages. In our microservices we have replaced Exporter class with our class for our needs. But with the new update there is a row, that breakes all our export features. \assert($exporter instanceof Exporter);

I suggest to remove this line or just check if $exporter is class. Of course, the best solution is to add Interface, don't you think?

VincentLanglet commented 1 year ago

You mean

$exporter = $this->container->get('sonata.exporter.exporter');
\assert($exporter instanceof Exporter);

?

I would be ok introducing an interface for the class https://github.com/sonata-project/exporter/blob/3.x/src/Exporter.php#L22

I would require to drop the 2.x support of sonata-exporter too which wouldn't be an issue https://github.com/sonata-project/SonataAdminBundle/blob/4.x/composer.json#L38

Do you want to do the PR on both sonata-exporter and sonata-admin @pkameisha ?

pkameisha commented 1 year ago

Yeah, that's what I mean

$exporter = $this->container->get('sonata.exporter.exporter');
\assert($exporter instanceof Exporter);

Sure, I can do PRs

VincentLanglet commented 1 year ago

3.1.0 is release so you can now do the sonataAdmin PR https://github.com/sonata-project/exporter/releases/tag/3.1.0

pkameisha commented 1 year ago

As I see, PR is merged. Could you create a new tag?

VincentLanglet commented 1 year ago

Done