Skip to content

NGSTACK-995 upgrade bundle to ibexa 5 #39

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

NGSTACK-995 upgrade bundle to ibexa 5 #39

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
f99fb0e
NGSTACK-995 update visitor-patterns argument to be of type CriterionI…
Oct 13, 2025
a4c5740
NGSTACK-995 remove unnecessary argument '' from methods and specify t…
Oct 13, 2025
92a3b90
NGSTACK-995 replace deprecated 'execute()' method with 'executeQuery'…
Oct 13, 2025
7cbbaf0
NGSTACK-995 remove colon in front of 'key' argument for 'setParameter…
Oct 13, 2025
7b561b3
NGSTACK-995 fix EnhancedSelectionStorageTest test
Oct 13, 2025
8fef5a4
NGSTACK-995 update composer.json with Ibexa v5 and add 'ibexa/solr' b…
Oct 13, 2025
63eab05
NGSTACK-995 remove 'ibexa-forms-bundle' from composer.json
Oct 16, 2025
49fe272
NGSTACK-995 apply php-cs-fixer to fix two files
Oct 16, 2025
7c30ee5
NGSTACK-995 fix tests and migrate phpunit.xml file to newer version
Oct 16, 2025
a0ddb36
NGSTACK-995 bump php version to 8.3 and symfony to 7.3 in 'tests.yml'…
Oct 16, 2025
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
9 changes: 2 additions & 7 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,10 @@ jobs:
strategy:
fail-fast: false
matrix:
php: ['7.4', '8.0', '8.1']
symfony: ['~5.4.0']
php: ['8.3']
symfony: ['~7.3.0']
phpunit: ['phpunit.xml']
deps: ['normal']
include:
- php: '7.4'
symfony: '~5.4.0'
phpunit: 'phpunit.xml'
deps: 'low'

steps:
- uses: actions/checkout@v2
Expand Down
10 changes: 5 additions & 5 deletions bundle/Command/MigrateCommand.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
namespace Netgen\Bundle\EnhancedSelectionBundle\Command;

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Driver\Result;
use Doctrine\DBAL\Result;
use Netgen\Bundle\EnhancedSelectionBundle\Core\FieldType\EnhancedSelection\Type as EnhancedSelectionType;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputInterface;
Expand Down Expand Up @@ -42,7 +42,7 @@ protected function initialize(InputInterface $input, OutputInterface $output): v
$this->io = new SymfonyStyle($input, $output);
}

protected function execute(InputInterface $input, OutputInterface $output): ?int
protected function execute(InputInterface $input, OutputInterface $output): int
{
$statement = $this->getFields();
$this->io->progressStart($statement->rowCount());
Expand Down Expand Up @@ -80,7 +80,7 @@ private function getFields(): Result
)
->setParameter('data_type_string', $this->typeIdentifier);

return $builder->execute();
return $builder->executeQuery();
}

private function resetFieldData(int $id, int $version): void
Expand All @@ -96,7 +96,7 @@ private function resetFieldData(int $id, int $version): void
->setParameter('id', $id)
->setParameter('version', $version);

$builder->execute();
$builder->executeStatement();
}

private function removeSelectionDataForField(int $id, int $version): void
Expand All @@ -111,7 +111,7 @@ private function removeSelectionDataForField(int $id, int $version): void
->setParameter('id', $id)
->setParameter('version', $version);

$builder->execute();
$builder->executeStatement();
}

private function createSelections(int $id, int $version, array $identifiers): void
Expand Down
15 changes: 11 additions & 4 deletions bundle/Core/FieldType/EnhancedSelection/EnhancedSelectionStorage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,17 @@
use Ibexa\Contracts\Core\FieldType\GatewayBasedStorage;
use Ibexa\Contracts\Core\Persistence\Content\Field;
use Ibexa\Contracts\Core\Persistence\Content\VersionInfo;
use Netgen\Bundle\EnhancedSelectionBundle\Core\FieldType\EnhancedSelection\EnhancedSelectionStorage\Gateway;

final class EnhancedSelectionStorage extends GatewayBasedStorage
{
public function storeFieldData(VersionInfo $versionInfo, Field $field, array $context): ?bool
public function __construct(Gateway $gateway)
{
$this->gateway = $gateway;
Copy link
Member

Choose a reason for hiding this comment

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

Is this a dynamic property?

Copy link
Author

Choose a reason for hiding this comment

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

In field_types.yaml file, the injected class here is DoctrineStorage, which extends an abstract class Gateway. I added the constructor here because of 'potentially polymorphic call` warning.

Copy link
Member

Choose a reason for hiding this comment

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

Where did the warning come from? The constructor is already in Ibexa\Contracts\Core\FieldType\GatewayBasedStorage abstract class and it doesn't make sense to repeat it here.

parent::__construct($gateway);
}

public function storeFieldData(VersionInfo $versionInfo, Field $field): ?bool
{
$this->gateway->deleteFieldData($versionInfo, [$field->id]);
if (!empty($field->value->externalData)) {
Expand All @@ -20,12 +27,12 @@ public function storeFieldData(VersionInfo $versionInfo, Field $field, array $co
return null;
}

public function getFieldData(VersionInfo $versionInfo, Field $field, array $context): void
public function getFieldData(VersionInfo $versionInfo, Field $field): void
{
$this->gateway->getFieldData($versionInfo, $field);
}

public function deleteFieldData(VersionInfo $versionInfo, array $fieldIds, array $context): bool
public function deleteFieldData(VersionInfo $versionInfo, array $fieldIds): bool
{
$this->gateway->deleteFieldData($versionInfo, $fieldIds);

Expand All @@ -37,7 +44,7 @@ public function hasFieldData(): bool
return true;
}

public function getIndexData(VersionInfo $versionInfo, Field $field, array $context)
public function getIndexData(VersionInfo $versionInfo, Field $field): bool
{
return false;
}
Expand Down
25 changes: 12 additions & 13 deletions bundle/Core/FieldType/EnhancedSelection/EnhancedSelectionStorage/Gateway/DoctrineStorage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Netgen\Bundle\EnhancedSelectionBundle\Core\FieldType\EnhancedSelection\EnhancedSelectionStorage\Gateway;

use Doctrine\DBAL\ArrayParameterType;
use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Types\Types;
use Ibexa\Contracts\Core\Persistence\Content\Field;
Expand Down Expand Up @@ -34,11 +35,11 @@ public function storeFieldData(VersionInfo $versionInfo, Field $field): void
'identifier' => ':identifier',
]
)
->setParameter(':contentobject_attribute_id', $field->id, Types::INTEGER)
->setParameter(':contentobject_attribute_version', $versionInfo->versionNo, Types::INTEGER)
->setParameter(':identifier', $identifier, Types::STRING);
->setParameter('contentobject_attribute_id', $field->id, Types::INTEGER)
->setParameter('contentobject_attribute_version', $versionInfo->versionNo, Types::INTEGER)
->setParameter('identifier', $identifier, Types::STRING);

$insertQuery->execute();
$insertQuery->executeStatement();
}
}

Expand All @@ -54,14 +55,14 @@ public function deleteFieldData(VersionInfo $versionInfo, array $fieldIds): void
->delete($this->connection->quoteIdentifier('sckenhancedselection'))
->where(
$query->expr()->and(
$query->expr()->in('contentobject_attribute_id', [':contentobject_attribute_id']),
$query->expr()->in('contentobject_attribute_id', ':contentobject_attribute_id'),
$query->expr()->eq('contentobject_attribute_version', ':contentobject_attribute_version')
)
)
->setParameter(':contentobject_attribute_id', $fieldIds, Connection::PARAM_INT_ARRAY)
->setParameter(':contentobject_attribute_version', $versionInfo->versionNo, Types::INTEGER);
->setParameter('contentobject_attribute_id', $fieldIds, ArrayParameterType::INTEGER)
->setParameter('contentobject_attribute_version', $versionInfo->versionNo, Types::INTEGER);

$query->execute();
$query->executeStatement();
}

/**
Expand All @@ -81,12 +82,10 @@ private function loadFieldData(int $fieldId, int $versionNo): array
$query->expr()->eq('contentobject_attribute_version', ':contentobject_attribute_version')
)
)
->setParameter(':contentobject_attribute_id', $fieldId, Types::INTEGER)
->setParameter(':contentobject_attribute_version', $versionNo, Types::INTEGER);
->setParameter('contentobject_attribute_id', $fieldId, Types::INTEGER)
->setParameter('contentobject_attribute_version', $versionNo, Types::INTEGER);

$statement = $query->execute();

$rows = $statement->fetchAllAssociative();
$rows = $query->executeQuery()->fetchAllAssociative();

return array_map(
static fn (array $row): string => $row['identifier'],
Expand Down
6 changes: 3 additions & 3 deletions bundle/Core/FieldType/EnhancedSelection/Type.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,9 +316,9 @@ public function validateFieldSettings($fieldSettings): array

if ($emptyCount > ($fieldSettings['isMultiple'] ? 0 : 1)) {
$validationErrors[] = new ValidationError(
$fieldSettings['isMultiple'] ?
"'%setting%' setting value item's 'identifier' property must have a value" :
"'%setting%' setting value item's 'identifier' property must have only a single empty value",
$fieldSettings['isMultiple']
? "'%setting%' setting value item's 'identifier' property must have a value"
: "'%setting%' setting value item's 'identifier' property must have only a single empty value",
null,
[
'%setting%' => $name,
Expand Down
22 changes: 18 additions & 4 deletions bundle/Core/Search/Legacy/Content/Common/Gateway/CriterionHandler/EnhancedSelection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,35 @@

use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Query\QueryBuilder;
use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion;
use Ibexa\Contracts\Core\Repository\Values\Content\Query\CriterionInterface;
use Ibexa\Core\Base\Exceptions\InvalidArgumentException;
use Ibexa\Core\Search\Legacy\Content\Common\Gateway\CriteriaConverter;
use Ibexa\Core\Search\Legacy\Content\Common\Gateway\CriterionHandler\FieldBase;
use Netgen\Bundle\EnhancedSelectionBundle\API\Repository\Values\Content\Query\Criterion\EnhancedSelection as EnhancedSelectionCriterion;

final class EnhancedSelection extends FieldBase
{
public function accept(Criterion $criterion): bool
public function accept(CriterionInterface $criterion): bool
{
return $criterion instanceof EnhancedSelectionCriterion;
}

public function handle(CriteriaConverter $converter, QueryBuilder $queryBuilder, Criterion $criterion, array $languageSettings): string
{
/**
Copy link
Member

Choose a reason for hiding this comment

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

superflous PHPDoc. This method does not throw anything and all @param and @return are already declared in method signature itself.

Copy link
Author

Choose a reason for hiding this comment

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

If you look at the method declaration in the abstract class, it has a PHPDoc which says that it throws an InvalidArgumentException. Also, in the method itself, the $this->getFieldDefinitionIds() method is called, which throws an InvalidArgumentException.

I added the PHPDoc first and foremost to say that the CriterionInterface param is actually EnhancedSelectionCriterion class. If that's alright with you, I would just keep this info in PHPDoc and remove everything else.

Copy link
Member

Choose a reason for hiding this comment

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

That's fine.

As for @throws, we should not specify those if the method does not directly throw. Otherwise, we might put every throw on every method ever :D

* @param CriteriaConverter $converter
* @param QueryBuilder $queryBuilder
* @param EnhancedSelectionCriterion $criterion
* @param array $languageSettings
*
* @return string
*
* @throws InvalidArgumentException
*/
public function handle(
CriteriaConverter $converter,
QueryBuilder $queryBuilder,
CriterionInterface $criterion,
array $languageSettings,
): string {
$fieldDefinitionIds = $this->getFieldDefinitionIds($criterion->target);

$subSelect = $this->connection->createQueryBuilder();
Expand Down
11 changes: 8 additions & 3 deletions bundle/Core/Search/Solr/Query/CriterionVisitor/EnhancedSelection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

namespace Netgen\Bundle\EnhancedSelectionBundle\Core\Search\Solr\Query\CriterionVisitor;

use Ibexa\Contracts\Core\Repository\Values\Content\Query\Criterion;
use Ibexa\Contracts\Core\Repository\Values\Content\Query\CriterionInterface;
use Ibexa\Contracts\Solr\Query\CriterionVisitor;
use Ibexa\Core\Base\Exceptions\InvalidArgumentException;
use Ibexa\Solr\Query\Common\CriterionVisitor\Field;
Expand All @@ -14,12 +14,17 @@

final class EnhancedSelection extends Field
{
public function canVisit(Criterion $criterion): bool
public function canVisit(CriterionInterface $criterion): bool
{
return $criterion instanceof EnhancedSelectionCriterion;
}

public function visit(Criterion $criterion, ?CriterionVisitor $subVisitor = null): string
/**
* @param EnhancedSelectionCriterion $criterion
Copy link
Member

Choose a reason for hiding this comment

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

Unneeded @param here.

Copy link
Author

Choose a reason for hiding this comment

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

The @param here is needed because it specifies that the CriterionInterface param is actually an instance of EnhancedSelectionCriterion class

*
* @throws InvalidArgumentException
Copy link
Member

Choose a reason for hiding this comment

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

We should use FQCN in PHPDocs.

Copy link
Author

Choose a reason for hiding this comment

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

May I ask why? I understand that there might be cases where it would be better (clearer) to use FQCN in PHPDocs, but here both classes are already imported and used elsewhere in this class.

Copy link
Member

Choose a reason for hiding this comment

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

Couple of reasons:

  1. It's our coding standard
  2. Consistency with cases where classes are not already imported
  3. It is much clearer and more readable to have a FQCN here, and it keeps code separate from comments.

*/
public function visit(CriterionInterface $criterion, ?CriterionVisitor $subVisitor = null): string
{
$searchFields = $this->getSearchFields($criterion);

Expand Down
6 changes: 3 additions & 3 deletions bundle/Form/Type/FieldType/FieldValueTransformer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ public function reverseTransform($value): Value
return $this->fieldType->getEmptyValue();
}

$hash = is_array($value['identifiers']) ?
$value['identifiers'] :
[$value['identifiers']];
$hash = is_array($value['identifiers'])
? $value['identifiers']
: [$value['identifiers']];

return $this->fieldType->fromHash($hash);
}
Expand Down
8 changes: 4 additions & 4 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,14 @@
}
],
"require": {
"ibexa/core": "^4.0",
"ibexa/content-forms": "^4.0",
"ibexa/core": "^5.0",
"ibexa/content-forms": "^5.0",
"twig/twig": "^3.0"
},
"require-dev": {
"phpunit/phpunit": "^9.0",
"netgen/ibexa-forms-bundle": "^4.0",
"friendsofphp/php-cs-fixer": "^3.3"
"friendsofphp/php-cs-fixer": "^3.3",
"ibexa/solr": "^5.0"
},
"minimum-stability": "dev",
"prefer-stable": true,
Expand Down
41 changes: 22 additions & 19 deletions phpunit.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<phpunit bootstrap="vendor/autoload.php"
<phpunit xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
bootstrap="vendor/autoload.php"
backupGlobals="false"
backupStaticAttributes="false"
convertErrorsToExceptions="true"
Expand All @@ -10,29 +11,31 @@
processIsolation="false"
stopOnFailure="false"
beStrictAboutTestsThatDoNotTestAnything="false"
>
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/9.3/phpunit.xsd">
<coverage>
<include>
<directory suffix=".php">bundle</directory>
</include>
<exclude>
<directory>bundle/DependencyInjection</directory>
<directory>bundle/Resources</directory>
<directory>vendor</directory>
<file>bundle/Core/Search/Legacy/Content/Common/Gateway/CriterionHandler/EnhancedSelection.php</file>
<file>bundle/Core/FieldType/EnhancedSelection/SearchField.php</file>
<file>bundle/NetgenEnhancedSelectionBundle.php</file>
</exclude>
<report>
<clover outputFile="build/logs/clover.xml"/>
<html outputDirectory="build/coverage"/>
<text outputFile="build/coverage.txt"/>
</report>
</coverage>
<testsuites>
<testsuite name="Netgen\EnhancedSelectionBundle\Tests">
<directory>tests</directory>
</testsuite>
</testsuites>
<filter>
<whitelist>
<directory suffix=".php">bundle</directory>
<exclude>
<directory>bundle/DependencyInjection</directory>
<directory>bundle/Resources</directory>
<directory>vendor</directory>
<file>bundle/Core/Search/Legacy/Content/Common/Gateway/CriterionHandler/EnhancedSelection.php</file>
<file>bundle/Core/FieldType/EnhancedSelection/SearchField.php</file>
<file>bundle/NetgenEnhancedSelectionBundle.php</file>
</exclude>
</whitelist>
</filter>
<logging>
<log type="junit" target="build/report.junit.xml"/>
<log type="coverage-html" target="build/coverage"/>
<log type="coverage-text" target="build/coverage.txt"/>
<log type="coverage-clover" target="build/logs/clover.xml"/>
<junit outputFile="build/report.junit.xml"/>
</logging>
</phpunit>
4 changes: 2 additions & 2 deletions tests/Core/FieldType/EnhancedSelection/EnhancedSelectionStorageTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@

namespace Netgen\Bundle\EnhancedSelectionBundle\Tests\Core\FieldType\EnhancedSelection;

use Ibexa\Contracts\Core\FieldType\StorageGateway;
use Ibexa\Contracts\Core\Persistence\Content\Field;
use Ibexa\Contracts\Core\Persistence\Content\FieldValue;
use Ibexa\Contracts\Core\Persistence\Content\VersionInfo;
use Ibexa\Core\FieldType\StorageGateway;
use Netgen\Bundle\EnhancedSelectionBundle\Core\FieldType\EnhancedSelection\EnhancedSelectionStorage;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;
Expand Down Expand Up @@ -43,7 +43,7 @@ public function testGetIndexData(): void
->disableOriginalConstructor()
->getMock();

self::assertFalse($this->storage->getIndexData($versionInfo, $field, []));
self::assertFalse($this->storage->getIndexData($versionInfo, $field));
}

public function testStoreFieldData(): void
Expand Down
4 changes: 1 addition & 3 deletions tests/Core/FieldType/EnhancedSelection/TypeTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,8 +375,6 @@ public function testAcceptValueWithValueObject(): void
{
$this->expectException(InvalidArgumentType::class);

$value = new Value([true, true]);

$this->type->acceptValue($value);
$this->type->acceptValue([true, true]);
}
}
2 changes: 1 addition & 1 deletion tests/Form/FieldTypeHandler/EnhancedSelectionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ public function testBuildFieldCreateForm(): void

$fieldDefinition = new FieldDefinition(
[
'id' => 'id',
'id' => 11,
'identifier' => 'identifier',
'isRequired' => true,
'descriptions' => ['fre-FR' => 'fre-FR'],
Expand Down
Loading