Skip to content
This repository was archived by the owner on Feb 4, 2023. It is now read-only.

Updated to support Symfony 5.0 & Twig 3.0 #930

Merged
merged 5 commits into from
Mar 23, 2020
Merged

Updated to support Symfony 5.0 & Twig 3.0 #930

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bc25e94
Updated to support Symfony 5.0 & Twig 3.0
Dec 9, 2019
f0cad58
Updated with additional dependencies (previously assumed would be inc…
Jan 7, 2020
526beff
Updated to use security-core package specifically
Jan 7, 2020
cb26da3
Updated PHPUnit (so a later version of symfony/yaml can also be used)
Jan 7, 2020
8ff49d5
Updated to remove type-hint, and use AbstractController instead
Jan 7, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .travis.yml
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,15 @@ matrix:
- php: 7.2
env: DEPENDENCIES=beta SYMFONY_REQUIRE="3.4.*"
- php: 7.2
env: DEPENDENCIES=beta SYMFONY_REQUIRE="4.3.*"
env: DEPENDENCIES=beta SYMFONY_REQUIRE="4.4.*"
- php: 7.2
env: DEPENDENCIES=beta SYMFONY_REQUIRE="5.0.*"
- php: 7.3
env: DEPENDENCIES=beta SYMFONY_REQUIRE="3.4.*"
- php: 7.3
env: DEPENDENCIES=beta SYMFONY_REQUIRE="4.3.*"
env: DEPENDENCIES=beta SYMFONY_REQUIRE="4.4.*"
- php: 7.3
env: DEPENDENCIES=beta SYMFONY_REQUIRE="5.0.*"
- php: hhvm
allow_failures:
- php: hhvm
Expand Down
4 changes: 2 additions & 2 deletions Controller/DatatableController.php
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@
use DateTime;
use Doctrine\DBAL\Types\Type;
use Exception;
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\PropertyAccess\PropertyAccess;
use Symfony\Component\PropertyAccess\PropertyAccessor;
use Symfony\Component\Routing\Annotation\Route;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;

class DatatableController extends Controller
class DatatableController extends AbstractController
{
//-------------------------------------------------
// Actions
Expand Down
15 changes: 10 additions & 5 deletions Datatable/AbstractDatatable.php
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@
use Symfony\Component\Routing\RouterInterface;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Translation\TranslatorInterface;
use Twig_Environment;
use Symfony\Component\Translation\TranslatorInterface as LegacyTranslatorInterface;
use Symfony\Contracts\Translation\TranslatorInterface;
use Twig\Environment;

abstract class AbstractDatatable implements DatatableInterface
{
Expand Down Expand Up @@ -62,7 +63,7 @@ abstract class AbstractDatatable implements DatatableInterface
/**
* The Twig Environment.
*
* @var Twig_Environment
* @var Environment
*/
protected $twig;

Expand Down Expand Up @@ -151,10 +152,10 @@ abstract class AbstractDatatable implements DatatableInterface
public function __construct(
AuthorizationCheckerInterface $authorizationChecker,
TokenStorageInterface $securityToken,
TranslatorInterface $translator,
$translator,
RouterInterface $router,
EntityManagerInterface $em,
Twig_Environment $twig
Environment $twig
) {
$this->validateName();

Expand All @@ -166,6 +167,10 @@ public function __construct(

$this->authorizationChecker = $authorizationChecker;
$this->securityToken = $securityToken;

if (!($translator instanceof LegacyTranslatorInterface) && !($translator instanceof TranslatorInterface)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure this is required?
I suppose this should work with both TranslatorInterface.
No need to force people to use Contracts

Copy link

Choose a reason for hiding this comment

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

Yes, it makes sure that you're getting either the deprecated interface from the component (available in Symfony 4.4 and earlier) or the newer interface from the contract (available for Symfony 4.3 and newer). The runtime check replaces the typehint that can't be enforced across multiple Symfony major versions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh yeah I didn't realize you added ! on both conditions. I thought you wanted to force the use of Contracts.

But since the translation interface param is injected with DI, I am not sure this check is very useful.

Copy link
Author

Choose a reason for hiding this comment

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

@Seb33300 the services.xml just passes in the @translator service ... so in 4.4 it'll be the previous class... in 5.0 it'll be the new contracts-interface.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I know and this means this cannot be something else than a TranslatorInterface.

The the check is quite useless, except if the developer override the configuration to pass something else.

If you really want to keep this check I think this is better to place it at the beginning of the method.

Copy link
Author

Choose a reason for hiding this comment

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

I don't think the contracts interface existed in 4.1 though; https://symfony.com/doc/4.1/translation.html

throw new \InvalidArgumentException(sprintf('The $translator argument of %s must be an instance of %s or %s, a %s was given.', static::class, LegacyTranslatorInterface::class, TranslatorInterface::class, get_class($translator)));
}
$this->translator = $translator;
$this->router = $router;
$this->em = $em;
Expand Down
4 changes: 2 additions & 2 deletions Datatable/Column/AbstractColumn.php
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
use Sg\DatatablesBundle\Datatable\OptionsTrait;
use Symfony\Component\OptionsResolver\OptionsResolver;
use Symfony\Component\Routing\RouterInterface;
use Twig_Environment;
use Twig\Environment;

abstract class AbstractColumn implements ColumnInterface
{
Expand Down Expand Up @@ -915,7 +915,7 @@ public function getTwig()
/**
* @return $this
*/
public function setTwig(Twig_Environment $twig)
public function setTwig(Environment $twig)
{
$this->twig = $twig;

Expand Down
4 changes: 2 additions & 2 deletions Datatable/Column/ColumnBuilder.php
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
use Exception;
use Sg\DatatablesBundle\Datatable\Factory;
use Symfony\Component\Routing\RouterInterface;
use Twig_Environment;
use Twig\Environment;

class ColumnBuilder
{
Expand Down Expand Up @@ -88,7 +88,7 @@ class ColumnBuilder
/**
* @param string $datatableName
*/
public function __construct(ClassMetadata $metadata, Twig_Environment $twig, RouterInterface $router, $datatableName, EntityManagerInterface $em)
public function __construct(ClassMetadata $metadata, Environment $twig, RouterInterface $router, $datatableName, EntityManagerInterface $em)
{
$this->metadata = $metadata;
$this->twig = $twig;
Expand Down
13 changes: 9 additions & 4 deletions Datatable/DatatableFactory.php
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@
use Symfony\Component\Routing\RouterInterface;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Translation\TranslatorInterface;
use Twig_Environment;
use Symfony\Component\Translation\TranslatorInterface as LegacyTranslatorInterface;
use Symfony\Contracts\Translation\TranslatorInterface;
use Twig\Environment;

class DatatableFactory
{
Expand Down Expand Up @@ -66,13 +67,17 @@ class DatatableFactory
public function __construct(
AuthorizationCheckerInterface $authorizationChecker,
TokenStorageInterface $securityToken,
TranslatorInterface $translator,
object $translator,
RouterInterface $router,
EntityManagerInterface $em,
Twig_Environment $twig
Environment $twig
) {
$this->authorizationChecker = $authorizationChecker;
$this->securityToken = $securityToken;

if (!($translator instanceof LegacyTranslatorInterface) && !($translator instanceof TranslatorInterface)) {
throw new \InvalidArgumentException(sprintf('The $translator argument of %s must be an instance of %s or %s, a %s was given.', static::class, LegacyTranslatorInterface::class, TranslatorInterface::class, get_class($translator)));
}
$this->translator = $translator;
$this->router = $router;
$this->em = $em;
Expand Down
2 changes: 1 addition & 1 deletion Tests/Column/ArrayColumnTest.php
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public function testArrayToString()
$arrayColumn = new ArrayColumn();
$result = $this->callMethod($arrayColumn, 'arrayToString', [['a', 'b' => ['d' => new \DateTime()]]]);
static::assertNotEmpty($result);
static::assertInternalType('string', $result);
static::assertIsString($result);
}

/**
Expand Down
6 changes: 3 additions & 3 deletions Tests/DatatableTest.php
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
use Symfony\Component\Routing\RouterInterface;
use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Translation\TranslatorInterface;
use Twig_Environment;
use Symfony\Contracts\Translation\TranslatorInterface;
use Twig\Environment;

/**
* @internal
Expand All @@ -38,7 +38,7 @@ public function testCreate()
/** @noinspection PhpUndefinedMethodInspection */
$router = $this->createMock(RouterInterface::class);
/** @noinspection PhpUndefinedMethodInspection */
$twig = $this->createMock(Twig_Environment::class);
$twig = $this->createMock(Environment::class);

/** @noinspection PhpUndefinedMethodInspection */
$em = $this->getMockBuilder(EntityManager::class)
Expand Down
2 changes: 1 addition & 1 deletion Tests/Response/DatatableQueryBuilderTest.php
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ final class DatatableQueryBuilderTest extends \PHPUnit\Framework\TestCase
/**
* {@inheritdoc}
*/
protected function setUp()
protected function setUp(): void
{
$this->entityManager = $this->prophesize(EntityManagerInterface::class);
$this->classMetadataFactory = $this->prophesize(ClassMetadataFactory::class);
Expand Down
32 changes: 16 additions & 16 deletions Twig/DatatableTwigExtension.php
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@
use Sg\DatatablesBundle\Datatable\Filter\FilterInterface;
use Symfony\Component\PropertyAccess\PropertyAccess;
use Symfony\Component\PropertyAccess\PropertyAccessor;
use Twig_Environment;
use Twig_Extension;
use Twig_SimpleFilter;
use Twig_SimpleFunction;
use Twig\Environment;
use Twig\Extension\AbstractExtension;
use Twig\TwigFilter;
use Twig\TwigFunction;

class DatatableTwigExtension extends Twig_Extension
class DatatableTwigExtension extends AbstractExtension
{
/**
* The PropertyAccessor.
Expand Down Expand Up @@ -55,27 +55,27 @@ public function getName()
public function getFunctions()
{
return [
new Twig_SimpleFunction(
new TwigFunction(
'sg_datatables_render',
[$this, 'datatablesRender'],
['is_safe' => ['html'], 'needs_environment' => true]
),
new Twig_SimpleFunction(
new TwigFunction(
'sg_datatables_render_html',
[$this, 'datatablesRenderHtml'],
['is_safe' => ['html'], 'needs_environment' => true]
),
new Twig_SimpleFunction(
new TwigFunction(
'sg_datatables_render_js',
[$this, 'datatablesRenderJs'],
['is_safe' => ['html'], 'needs_environment' => true]
),
new Twig_SimpleFunction(
new TwigFunction(
'sg_datatables_render_filter',
[$this, 'datatablesRenderFilter'],
['is_safe' => ['html'], 'needs_environment' => true]
),
new Twig_SimpleFunction(
new TwigFunction(
'sg_datatables_render_multiselect_actions',
[$this, 'datatablesRenderMultiselectActions'],
['is_safe' => ['html'], 'needs_environment' => true]
Expand All @@ -89,7 +89,7 @@ public function getFunctions()
public function getFilters()
{
return [
new Twig_SimpleFilter('sg_datatables_bool_var', [$this, 'boolVar']),
new TwigFilter('sg_datatables_bool_var', [$this, 'boolVar']),
];
}

Expand All @@ -102,7 +102,7 @@ public function getFilters()
*
* @return string
*/
public function datatablesRender(Twig_Environment $twig, DatatableInterface $datatable)
public function datatablesRender(Environment $twig, DatatableInterface $datatable)
{
return $twig->render(
'@SgDatatables/datatable/datatable.html.twig',
Expand All @@ -117,7 +117,7 @@ public function datatablesRender(Twig_Environment $twig, DatatableInterface $dat
*
* @return string
*/
public function datatablesRenderHtml(Twig_Environment $twig, DatatableInterface $datatable)
public function datatablesRenderHtml(Environment $twig, DatatableInterface $datatable)
{
return $twig->render(
'@SgDatatables/datatable/datatable_html.html.twig',
Expand All @@ -132,7 +132,7 @@ public function datatablesRenderHtml(Twig_Environment $twig, DatatableInterface
*
* @return string
*/
public function datatablesRenderJs(Twig_Environment $twig, DatatableInterface $datatable)
public function datatablesRenderJs(Environment $twig, DatatableInterface $datatable)
{
return $twig->render(
'@SgDatatables/datatable/datatable_js.html.twig',
Expand All @@ -149,7 +149,7 @@ public function datatablesRenderJs(Twig_Environment $twig, DatatableInterface $d
*
* @return string
*/
public function datatablesRenderFilter(Twig_Environment $twig, DatatableInterface $datatable, ColumnInterface $column, $position)
public function datatablesRenderFilter(Environment $twig, DatatableInterface $datatable, ColumnInterface $column, $position)
{
/** @var FilterInterface $filter */
$filter = $this->accessor->getValue($column, 'filter');
Expand Down Expand Up @@ -181,7 +181,7 @@ public function datatablesRenderFilter(Twig_Environment $twig, DatatableInterfac
*
* @return string
*/
public function datatablesRenderMultiselectActions(Twig_Environment $twig, ColumnInterface $multiselectColumn, $pipeline)
public function datatablesRenderMultiselectActions(Environment $twig, ColumnInterface $multiselectColumn, $pipeline)
{
$parameters = [];
$values = [];
Expand Down
19 changes: 12 additions & 7 deletions composer.json
100644 → 100755
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,20 @@
"php": ">=7.1",
"doctrine/orm": "^2.5",
"friendsofsymfony/jsrouting-bundle": "^1.6|^2.0",
"symfony/framework-bundle": "^3.4|^4.1",
"symfony/options-resolver": "^3.4|^4.1",
"symfony/property-access": "^3.4|^4.1",
"symfony/security": "^3.4|^4.1",
"symfony/translation": "^3.4|^4.1",
"twig/twig": "^2.9"
"symfony/config": "^3.4|^4.1|^5.0",
"symfony/dependency-injection": "^3.4|^4.1|^5.0",
"symfony/http-foundation": "^3.4|^4.1|^5.0",
"symfony/http-kernel": "^3.4|^4.1|^5.0",
"symfony/framework-bundle": "^3.4|^4.1|^5.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you know if there is any reason to not be compatible with Symfony 4.0?

Copy link
Collaborator

@Seb33300 Seb33300 Jan 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Change made here for security reasons: #885

Not sure if its the responsibility of this bundle to force people to upgrade because this version of symfony is not maintained anymore.
IMH we should keep compatibility with 4.0 and let people upgrade when they can.

Copy link
Author

Choose a reason for hiding this comment

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

Dunno, but it's a bit out of scope for this PR ... i've only just expanded the existing composer requirements to 5.0 and added the extra components based on the existing 4.1 requirements for the existing components.

"symfony/options-resolver": "^3.4|^4.1|^5.0",
"symfony/property-access": "^3.4|^4.1|^5.0",
"symfony/routing": "^3.4|^4.1|^5.0",
"symfony/security-core": "^3.4|^4.1|^5.0",
"symfony/translation": "^3.4|^4.1|^5.0",
"twig/twig": "^2.9|^3.0"
},
"require-dev": {
"phpunit/phpunit": "5.7.*",
"phpunit/phpunit": "^7.5|^8.5",
"friendsofphp/php-cs-fixer": "^2.15"
},
"autoload": {
Expand Down