fix composer.json

[DonCallisto]

There's a problem with travis matrix file I suppose. Can you take a look at this and fix it? Thanks.

[tugmaks]

done

[romainguerrero]

Hope this PR will be accepted soon as we love this bundle but couldn't use it on our new Symfony 4 based project...

[DonCallisto]

I really don't like this sequence of if(s). There's not better solution for this?

[DonCallisto]

@tugmaks way better. Listen, could you the require-dev modifications? In particular I can't understand the symfony/framework part.

[tugmaks]

@DonCallisto, sorry i do not understand your question. You mean why i included symfony/framework-bundle to require-dev section ?

[DonCallisto]

Yes (and also explain others you added). I'm not saying is wrong, just wanted to double check with you.

[tugmaks]

@DonCallisto OK, let's check it again:

Require section

package	usage example
symfony/form	https://github.com/APY/APYDataGridBundle/blob/master/Grid/Column/NumberColumn.php#L15
symfony/dependency-injection	https://github.com/APY/APYDataGridBundle/blob/master/DependencyInjection/APYDataGridExtension.php#L16
symfony/config	https://github.com/APY/APYDataGridBundle/blob/master/DependencyInjection/APYDataGridExtension.php#L15
symfony/http-foundation	https://github.com/APY/APYDataGridBundle/blob/master/Grid/GridManager.php#L15
symfony/http-kernel	https://github.com/APY/APYDataGridBundle/blob/master/APYDataGridBundle.php#L18
symfony/options-resolver	https://github.com/APY/APYDataGridBundle/blob/master/Grid/AbstractType.php#L5
symfony/security	https://github.com/APY/APYDataGridBundle/blob/master/Grid/Columns.php#L18
symfony/serializer	https://github.com/APY/APYDataGridBundle/blob/master/Grid/Export/XMLExport.php#L17
symfony/templating	No hard deps. Should be moved to require-dev or test will fail, because EngineInterface extends Symfony\Component\Templating\EngineInterface

Require-dev

package	usage example
symfony/framework-bundle	https://github.com/APY/APYDataGridBundle/blob/master/Tests/Grid/GridTest.php#L21
symfony/browser-kit	https://github.com/APY/APYDataGridBundle/blob/master/Tests/Grid/GridManagerTest.php#L9
symfony/expression-language	There was some problems without this package https://travis-ci.org/APY/APYDataGridBundle/jobs/389112843 but right now if i run tests locally all tests are passed. We can remove it and see what will happen in travis

[DonCallisto]

Ok, thank you for this recap. Please, try to remove expression-language and check if tests pass. If them pass, I merge the PR.

[tugmaks]

The same story, build passed but many warnings

1) APY\DataGridBundle\Tests\Grid\Column\ArrayColumnTest::testRenderCellWithoutCallback Cannot mock Symfony\Component\Routing\Router::addExpressionLanguageProvider() because a class or interface used in the signature is not loade https://github.com/symfony/symfony/issues/14376

[DonCallisto]

That's strange: PHPUnit still signal warning but https://github.com/symfony/routing/blob/master/composer.json#L25 include expression-language. It should be included by router itself, right?

[tugmaks]

it is in require-dev of router

[DonCallisto]

Ok, I think it should be acceptable - at least for the moment - to have expression-language as a dev dependency. At least we can release it for sf4 users.

[tugmaks]

ok, i will move expression-language back to require-dev

[DonCallisto]

@tugmaks thanks!

[DonCallisto]

@romainguerrero before drafting a new release, could you try the master branch in your project and let me know if anything breaks? Thanks.

[tugmaks]

@DonCallisto thank you too!

[Scuottolinx]

@romainguerrero have a news?

[DonCallisto]

@Scuottolinx you can test this aswell: just require master in your composer.json and see if everything's good

[DonCallisto]

@tugmaks you have tried it? Is it fine?

[romainguerrero]

@DonCallisto yes we have used it in our Symfony 4.1 based project requiring the 3.2.x-dev tag. We encourred a problem for injecting the Grid ou GridManager in a custom app service and were forced to redefine the grid or the grid.manager alias like this :

# config/services.yaml

    APY\DataGridBundle\Grid\GridManager:
        alias: grid.manager

I wanted to search why we had to do it for this bundle, but I hadn't enought time for those past days... But the bundle seams to do the job perfectly :)

[DonCallisto]

https://github.com/APY/APYDataGridBundle/pull/1019 should fix it, right? Apart that problem? Any other issues?

[romainguerrero]

@DonCallisto unfortunatly no (we were using my own branch until you merge it)... And for the moment I don't understand why :confused: But we encourred no other problem for the moment :slightly_smiling_face:

[DonCallisto]

So even if you're using the new merged version, getting rid of your fork, the problem is there? Could you let me know which problem is (maybe a stack trace or something similar). Thanks.

[romainguerrero]

@DonCallisto of course ! If I removed the lines in my config/services.yaml I have this issue :

Symfony\Component\DependencyInjection\Exception\RuntimeException:
Cannot resolve argument $gridService of "App\Controller\Admin\Administrator\AdministratorController::index()": Cannot autowire service "App\Service\GridService": argument "$gridManager" of method "__construct()" references class "APY\DataGridBundle\Grid\GridManager" but no such service exists. You should maybe alias this class to the existing "grid.manager" service.

  at var/cache/dev/ContainerXXrP5Tx/getGridServiceService.php:9
  at require()
     (var/cache/dev/ContainerXXrP5Tx/srcDevDebugProjectContainer.php:399)
  at ContainerXXrP5Tx\srcDevDebugProjectContainer->load('getGridServiceService.php')
     (var/cache/dev/ContainerXXrP5Tx/get_ServiceLocator__A5qa_NService.php:10)
  at ContainerXXrP5Tx\srcDevDebugProjectContainer->{closure}()
     (vendor/symfony/dependency-injection/ServiceLocator.php:64)
  at Symfony\Component\DependencyInjection\ServiceLocator->get('gridService')
     (vendor/symfony/http-kernel/Controller/ArgumentResolver/ServiceValueResolver.php:68)
  at Symfony\Component\HttpKernel\Controller\ArgumentResolver\ServiceValueResolver->resolve(object(Request), object(ArgumentMetadata))
     (vendor/symfony/http-kernel/Controller/ArgumentResolver/TraceableValueResolver.php:58)
  at Symfony\Component\HttpKernel\Controller\ArgumentResolver\TraceableValueResolver->resolve(object(Request), object(ArgumentMetadata))
     (vendor/symfony/http-kernel/Controller/ArgumentResolver.php:62)
  at Symfony\Component\HttpKernel\Controller\ArgumentResolver->getArguments(object(Request), array(object(AdministratorController), 'index'))
     (vendor/symfony/http-kernel/Controller/TraceableArgumentResolver.php:38)
  at Symfony\Component\HttpKernel\Controller\TraceableArgumentResolver->getArguments(object(Request), array(object(AdministratorController), 'index'))
     (vendor/symfony/http-kernel/HttpKernel.php:141)
  at Symfony\Component\HttpKernel\HttpKernel->handleRaw(object(Request), 1)
     (vendor/symfony/http-kernel/HttpKernel.php:66)
  at Symfony\Component\HttpKernel\HttpKernel->handle(object(Request), 1, true)
     (vendor/symfony/http-kernel/Kernel.php:188)
  at Symfony\Component\HttpKernel\Kernel->handle(object(Request))
     (public/index.php:37)

And here is my service definition :

namespace App\Service;

...
use APY\DataGridBundle\Grid\Grid;
use APY\DataGridBundle\Grid\GridManager;
use APY\DataGridBundle\Grid\Source\Entity;

class GridService
{
    private $gridManager;

    public function __construct(GridManager $gridManager)
    {
        $this->gridManager = $gridManager;
    }

[DonCallisto]

Yes, of course, now I got it. We should define those alias in order to guarantee backward compatibility and in the same time use FQCN as ID(s) to define the services.

Thanks for pointing it out, I'll work on it ASAP.