Skip to content

IBX-11833: Added a unique field-type option to prevent duplicate Meta fields #2012

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
mateuszdebinski wants to merge 1 commit into
base: 4.6
Choose a base branch
from
Open

IBX-11833: Added a unique field-type option to prevent duplicate Meta fields #2012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 0 additions & 30 deletions phpstan-baseline.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4308,12 +4308,6 @@ parameters:
count: 1
path: src/lib/Component/TwigComponent.php

-
message: '#^Offset ''meta'' might not exist on array\{position\: int, meta\?\: bool\}\.$#'
identifier: offsetAccess.notFound
count: 1
path: src/lib/Config/AdminUiForms/ContentTypeFieldTypesResolver.php

-
message: '#^Method Ibexa\\AdminUi\\Event\\Options\:\:__construct\(\) has parameter \$options with no value type specified in iterable type array\.$#'
identifier: missingType.iterableValue
Expand Down Expand Up @@ -11687,12 +11681,6 @@ parameters:
count: 1
path: src/lib/Service/ContentTypeService.php

-
message: '#^Access to an undefined property Ibexa\\Contracts\\Core\\Repository\\Values\\ValueObject\:\:\$fieldDefinitions\.$#'
identifier: property.notFound
count: 1
path: src/lib/Service/MetaFieldType/MetaFieldDefinitionService.php

-
message: '#^Access to an undefined property Ibexa\\Contracts\\Core\\Repository\\Values\\User\\Policy\:\:\$originalId\.$#'
identifier: property.notFound
Expand Down Expand Up @@ -14596,24 +14584,6 @@ parameters:
count: 1
path: tests/lib/Config/AdminUiForms/ContentTypeFieldTypesResolverTest.php

-
message: '#^Method Ibexa\\Tests\\AdminUi\\Config\\AdminUiForms\\ContentTypeFieldTypesResolverTest\:\:testGetMetaFieldTypes\(\) has parameter \$expectedMetaFieldTypes with no value type specified in iterable type array\.$#'
identifier: missingType.iterableValue
count: 1
path: tests/lib/Config/AdminUiForms/ContentTypeFieldTypesResolverTest.php

-
message: '''
#^PHPDoc tag @param has invalid value \(array\<string, array\{
'meta'\: bool,
'position'\: int,
\}\>
\$expectedMetaFieldTypes\)\: Unexpected token "\\n \* ", expected variable at offset 279 on line 11$#
'''
identifier: phpDoc.parseError
count: 1
path: tests/lib/Config/AdminUiForms/ContentTypeFieldTypesResolverTest.php

-
message: '#^Call to static method PHPUnit\\Framework\\Assert\:\:assertNull\(\) with Symfony\\Component\\HttpFoundation\\Response will always evaluate to false\.$#'
identifier: staticMethod.impossibleType
Expand Down
4 changes: 4 additions & 0 deletions src/bundle/DependencyInjection/Configuration/Parser/AdminUiForms.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,10 @@ static function (array $config): bool {
->defaultFalse()
->end()
->integerNode('position')->end()
->booleanNode('unique')
->info('Prevent this field_type from being added more than once per Content Type, regardless of the field group it already exists in')
->defaultFalse()
->end()
->end()
->end()
->end()
Expand Down
3 changes: 2 additions & 1 deletion src/lib/Config/AdminUiForms/ContentTypeFieldTypesResolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ public function __construct(ConfigResolverInterface $configResolver)
* @return array<string, array{
* 'position': int,
* 'meta'?: bool,
* 'unique': bool,
* }>
*/
public function getFieldTypes(): array
Expand All @@ -43,7 +44,7 @@ public function getMetaFieldTypes(): array
$fieldTypes = $this->getFieldTypes();
$metaFieldTypes = array_filter(
$fieldTypes,
static fn (array $config): bool => true === $config['meta']
static fn (array $config): bool => true === ($config['meta'] ?? false)
);

$positions = array_column($metaFieldTypes, 'position');
Expand Down
2 changes: 2 additions & 0 deletions src/lib/Config/AdminUiForms/ContentTypeFieldTypesResolverInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ interface ContentTypeFieldTypesResolverInterface
/**
* @return array<string, array{
* 'meta'?: bool,
* 'unique'?: bool,
* 'position'?: int,
* }>
*/
Expand All @@ -25,6 +26,7 @@ public function getFieldTypes(): array;
* @return array<string, array{
* 'meta': bool,
* 'position': int,
* 'unique': bool,
* }>
*/
public function getMetaFieldTypes(): array;
Expand Down
13 changes: 10 additions & 3 deletions src/lib/Service/MetaFieldType/MetaFieldDefinitionService.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,9 @@ public function __construct(
$this->translator = $translator;
}

/**
* @param \Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeCreateStruct|\Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeDraft $contentType
*/
public function addMetaFieldDefinitions(ValueObject $contentType, ?Language $language = null): void
{
$metaFieldTypes = $this->contentTypeFieldTypesResolver->getMetaFieldTypes();
Expand All @@ -70,8 +73,9 @@ public function addMetaFieldDefinitions(ValueObject $contentType, ?Language $lan

foreach ($metaFieldTypes as $metaFieldTypeIdentifier => $metaFieldTypeSettings) {
$fieldGroup = $this->getDefaultMetaDataFieldTypeGroup() ?? $this->fieldsGroupsList->getDefaultGroup();
$fieldTypeGroup = $metaFieldTypeSettings['unique'] ? null : $fieldGroup;

@barw4 barw4 Aug 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we make stricter comparison here?


if ($this->metaFieldDefinitionExists($metaFieldTypeIdentifier, $fieldGroup, $contentType)) {
if ($this->metaFieldDefinitionExists($metaFieldTypeIdentifier, $fieldTypeGroup, $contentType)) {
continue;
}

Expand Down Expand Up @@ -120,15 +124,18 @@ public function createMetaFieldDefinitionCreateStruct(
return $fieldDefinitionCreateStruct;
}

/**
* @param \Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeCreateStruct|\Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeDraft $contentType
*/
public function metaFieldDefinitionExists(
string $fieldTypeIdentifier,
string $fieldTypeGroup,
?string $fieldTypeGroup,
ValueObject $contentType
): bool {
foreach ($contentType->fieldDefinitions as $fieldDefinition) {
if (
$fieldDefinition->fieldTypeIdentifier === $fieldTypeIdentifier
&& $fieldDefinition->fieldGroup === $fieldTypeGroup
&& ($fieldTypeGroup === null || $fieldDefinition->fieldGroup === $fieldTypeGroup)
) {
return true;
}
Expand Down
8 changes: 7 additions & 1 deletion src/lib/Service/MetaFieldType/MetaFieldDefinitionServiceInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
*/
interface MetaFieldDefinitionServiceInterface
{
/**
* @param \Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeCreateStruct|\Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeDraft $contentType
*/
public function addMetaFieldDefinitions(
ValueObject $contentType,
?Language $language = null
Expand All @@ -29,9 +32,12 @@ public function createMetaFieldDefinitionCreateStruct(
int $position
): FieldDefinitionCreateStruct;

/**
* @param \Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeCreateStruct|\Ibexa\Contracts\Core\Repository\Values\ContentType\ContentTypeDraft $contentType
*/
public function metaFieldDefinitionExists(
string $fieldTypeIdentifier,
string $fieldTypeGroup,
?string $fieldTypeGroup,
ValueObject $contentType
): bool;

Expand Down
91 changes: 91 additions & 0 deletions ...ndle/DependencyInjection/Configuration/Parser/AdminUiFormsFieldTypesConfigurationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
<?php

/**
* @copyright Copyright (C) Ibexa AS. All rights reserved.
* @license For full copyright and license information view LICENSE file distributed with this source code.
*/
declare(strict_types=1);

namespace Ibexa\Tests\Bundle\AdminUi\DependencyInjection\Configuration\Parser;

use Ibexa\Bundle\AdminUi\DependencyInjection\Configuration\Parser\AdminUiForms;
use PHPUnit\Framework\TestCase;
use Symfony\Component\Config\Definition\Builder\TreeBuilder;
use Symfony\Component\Config\Definition\Processor;

final class AdminUiFormsFieldTypesConfigurationTest extends TestCase
{
private Processor $processor;

protected function setUp(): void
{
$this->processor = new Processor();
}

public function testMetaAndUniqueDefaultToFalseWhenNotConfigured(): void
{
$result = $this->processFieldTypes([
'plain_field' => [],
]);

self::assertSame(
['meta' => false, 'unique' => false],
$result['plain_field']
);
}

public function testUniqueCanBeExplicitlyEnabledForAMetaFieldType(): void
{
$result = $this->processFieldTypes([
'seo_metadata' => ['meta' => true, 'position' => 1, 'unique' => true],
]);

self::assertSame(
['meta' => true, 'position' => 1, 'unique' => true],
$result['seo_metadata']
);
}

public function testPositionIsRequiredForMetaFieldTypes(): void
{
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage('The "position" option is required for all Meta Field Types');

$this->processFieldTypes([
'seo_metadata' => ['meta' => true],
]);
}

public function testPositionIsNotRequiredForNonMetaFieldTypes(): void
{
$result = $this->processFieldTypes([
'plain_field' => ['meta' => false],
]);

self::assertArrayNotHasKey('position', $result['plain_field']);
}

/**
* @param array<string, array<string, mixed>> $fieldTypes
*
* @return array<string, array<string, mixed>>
*/
private function processFieldTypes(array $fieldTypes): array
{
$treeBuilder = new TreeBuilder('admin_ui_forms_test');
$nodeBuilder = $treeBuilder->getRootNode()->children();
(new AdminUiForms())->addSemanticConfig($nodeBuilder);

$config = $this->processor->process($treeBuilder->buildTree(), [
[
'admin_ui_forms' => [
'content_type_edit' => [
'field_types' => $fieldTypes,
],
],
],
]);

return $config['admin_ui_forms']['content_type_edit']['field_types'];
}
}
13 changes: 11 additions & 2 deletions tests/lib/Config/AdminUiForms/ContentTypeFieldTypesResolverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,13 @@ public function testGetFieldTypes(bool $hasParameter, array $expectedFieldTypes)
* @param array<string, array{
* 'position'?: int,
* 'meta'?: bool,
* 'unique'?: bool,
* }> $fieldTypes
* @param array<string, array{
* 'meta': bool,
* 'position': int,
* }>
* $expectedMetaFieldTypes
* 'unique': bool,
* }> $expectedMetaFieldTypes
*/
public function testGetMetaFieldTypes(
bool $hasParameter,
Expand All @@ -83,6 +84,7 @@ public function testGetMetaFieldTypes(
* @param array<string, array{
* 'meta'?: bool,
* 'position'?: int,
* 'unique'?: bool,
* }> $metaFieldTypes
* @param array<string> $expectedIdentifiers
*/
Expand Down Expand Up @@ -138,10 +140,12 @@ public function provideDataForTestGetFieldTypes(): iterable
* array<string, array{
* 'meta'?: bool,
* 'position'?: int,
* 'unique'?: bool,
* }>,
* array<string, array{
* 'meta': bool,
* 'position': int,
* 'unique': bool,
* }>
* }>
*/
Expand All @@ -156,11 +160,13 @@ public function provideDataForTestGetMetaFieldTypes(): iterable
$foo = [
'meta' => true,
'position' => 2,
'unique' => false,
];

$bar = [
'meta' => true,
'position' => 10,
'unique' => true,
];

yield [
Expand All @@ -185,6 +191,7 @@ public function provideDataForTestGetMetaFieldTypes(): iterable
* array<string, array{
* 'meta': bool,
* 'position': int,
* 'unique': bool,
* }>,
* array<string>
* }>
Expand All @@ -203,10 +210,12 @@ public function provideDataForTestGetMetaFieldTypeIdentifiers(): iterable
'foo' => [
'meta' => true,
'position' => 1,
'unique' => false,
],
'bar' => [
'meta' => true,
'position' => 2,
'unique' => false,
],
],
['foo', 'bar'],
Expand Down
Loading
Loading