IBX-11833: Added check if field is singular to prevent add to CT - #2060
mateuszdebinski wants to merge 2 commits into
Conversation
konradoboza
left a comment
konradoboza
left a comment
There was a problem hiding this comment.
Please check if cases in tests/lib/Service/MetaFieldType/MetaFieldDefinitionServiceTest.php can be merged into dataProvider as it seems the major part of those is duplicated.
9b63bba to
e8920a2
Compare
e8920a2 to
9f38089
Compare
|
| $fieldGroup = $this->getDefaultMetaDataFieldTypeGroup() ?? $this->fieldsGroupsList->getDefaultGroup(); | ||
| try { | ||
| $isSingular = $this->fieldTypeService->getFieldType($metaFieldTypeIdentifier)->isSingular(); | ||
| } catch (NotFoundException $e) { |
There was a problem hiding this comment.
It looks like getFieldType() real implementation may throw Ibexa\Core\Base\Exceptions\NotFound\FieldTypeNotFoundException and extends \RuntimeException rather than Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException and extends \Exception. At least based on FieldTypeRegistry::getFieldType() in ibexa/core 4.6 branch. If that's correct, this catch may not actually catch the case it's meant to handle. Could we check which exception is thrown here, and if it's FieldTypeNotFoundException, catch that one too or adjust the catch clause? It may also be worth to check the test double in MetaFieldDefinitionServiceTest against the exception the real service throws, just to be safe.
5 participants
Related PRs:
Description:
A field type could get duplicated into a Content Type when its resolved field group differed from where an existing instance already lived (e.g., SEO metadata re-added on edit). The SEO field type has an isSingular function that returns true, which means that the field can only be added once to the CT.
This PR introduces verification if field is singular before adding metafields to the CT, including the SEO field.
For QA:
Documentation: