Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
BlockFieldItem currently doesn't provide a custom field normalizer. Having no custom field normalizer can cause issues when we want to normalize the block field item values. One use case would be, exporting default content for example.
The issue because a problem when using a "settings" field property (MapDataDefinition) which doesn't support normalization at all.
Proposed resolution
Create a custom block field item normalizer and expose it in case there is a hal module available.
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#8 | interdiff-2882727-5-8.txt | 2 KB | John Cook |
#8 | interdiff-2882727-2-8.txt | 1.77 KB | John Cook |
#8 | add_custom_hal_normalizer-2882727-8.patch | 2.4 KB | John Cook |
#2 | add_custom_hal_normalizer-2882727-2.patch | 2.31 KB | mbovan |
|
Comments
Comment #2
mbovan CreditAttribution: mbovan at MD Systems GmbH commentedAdding a patch that provides normalization for the settings field property as well.
Comment #3
Wim Leersjson
,xml
, etc)?Documenting the reason for this priority is useful for future maintainability.
At least until #2575761: Discuss a better system for discovering and selecting normalizers is fixed.
{@inheritdoc}
s/HAL normalizer/HAL field item normalizer/
Hrm, what is this actually normalizing?
If there were tests, this would actually be clear.
Comment #4
Wim LeersFor an example for tests, see
\Drupal\Tests\serialization\Unit\Normalizer\TimestampItemNormalizerTest
+ ideally also integration tests (for which I can also find you an example), but having at least such a unit test would be very valuable.(Because
TimestampItemNormalizer extends FieldItemNormalizer
, just like this.)Comment #5
John Cook CreditAttribution: John Cook at Creode commentedI've moved the service provider definition into services.yml instead of a custom class.
Changed the comments as suggested by Wim in #3.
@Wim; creating at test is causing be a bit of trouble. If I mock
BlockFieldItem
, as withTimestampItemNormalizerTest
, then the normalizer can't get thesettings
property. I've also tried to instantiateBlockFieldItem
mocking other things but I'm having trouble getting it to work; if you know of anywhere I can see an example that would be helpful.Further detail about mocking
Using
$field_item->settings
:BlockFieldItem
returns an array as expected using it's magic getter.NULL
, probably because the property doesn't exist on the mock.Using a
foreach ($field_item as $property_name => $property)
loop:BlockFieldItem
returns a\Drupal\Core\TypedData\Plugin\DataType\Map
object.I could put in a check, but wouldn't the mean that the code would not be checked correctly?
Comment #7
BerdirThe service provider is required because this module also needs to work without the hal module
Comment #8
John Cook CreditAttribution: John Cook at Creode commentedThanks Berdir. I've reverted to use the custom service provider. There's interdiffs of this patch with the patches int #2 and #5.
Comment #10
John Cook CreditAttribution: John Cook at Creode commentedNow I've got no idea what testbot wants.
It works locally.
Comment #11
BerdirTests are run against 8.4, you are likely on 8.3.
Try removing the administer nodes permission then the UI is the same on 8.4 and 8.3. But that should be a separate issue, that's an existing fail.
Comment #12
JamesK CreditAttribution: JamesK at Advisor Websites commentedWhy does it depend on hal rather than serialization? You should be able to extend Drupal\serialization\Normalizer\FieldItemNormalizer to use this more generically.
Also, this is likely to require follow-up after #2895532: @DataType=map cannot be normalized, affects @FieldType=link, @FieldType=map
Comment #13
JamesK CreditAttribution: JamesK at Advisor Websites commentedAs the patch is now in #2895532: @DataType=map cannot be normalized, affects @FieldType=link, @FieldType=map (#96) it actually fixes this issue without anything needing to happen here.
Comment #14
BerdirAgreed, closing as a duplicate of the core issue.