Add tests and documentation updates for StructBlock translatable_blocks support (based on #752)#882
Add tests and documentation updates for StructBlock translatable_blocks support (based on #752)#882Andreew4x4 wants to merge 8 commits intowagtail:mainfrom
Conversation
- Remove override_translatable_blocks section from field-configuration.md - Update test docstring to reflect actual implementation - Feature was documented but not implemented in extract.py The override_translatable_blocks feature was documented in the how-to guide but is not actually implemented in the extraction code. Removing this section to avoid user confusion until the feature is fully implemented. This change ensures documentation accurately describes only the implemented translatable_blocks feature.
- Extract segments from synchronized StreamFields (was returning 0 segments) - CharBlock/TextBlock return OverridableSegmentValue in synchronized fields - Add is_synchronized_field parameter to distinguish explicit SynchronizedField - Fix test attribute: s.data -> s.string
There was a problem hiding this comment.
Hi @Andreew4x4 ,I reviewed the PR and tested the implementation locally.
Here's my review:
This looks solid. The core implementation is clean and backward compatible, the tests are comprehensive, and the documentation is thorough with practical examples. I appreciate the variety of test scenarios (YouTubeBlock, CodeBlock, AddressBlock) as they make it easy to understand the use cases.
A few observations:
-
Silent failure on typos: If a developer misspells a field name in translatable_blocks (e.g.,
["decription"]instead of["description"]), the field is silently excluded. Would it be worth logging a warning when a field name in translatable_blocks doesn't match any child block? This could save debugging time. -
Interaction with get_translatable_segments(): I noticed that if a StructBlock defines both translatable_blocks and
get_translatable_segments(), the latter takes priority since it's checked before the StructBlock handler is reached. It might be helpful to add a note about this in the documentation, so developers know which one "wins." -
Nested translatable_blocks test: The nested StructBlock test (test_nested_structblock_with_translatable_blocks) tests a StreamBlock nested inside a page, but it doesn't test a StructBlock-inside-StructBlock where both levels have different translatable_blocks. Would that be a useful additional test case?
These are minor observations. the PR looks good to merge as-is. Great work completing the long-standing request from issue #307!
4 participants
This PR adding comprehensive tests and documentation improvements for the already-implemented translatable_blocks feature on the open #752, which addresses issue #307.
The feature allows developers to control which sub-fields of a StructBlock are translatable by defining a translatable_blocks attribute on the block class.
The core implementation changes were introduced in the cherry-picked commits (6835d9d, 417b8de, 91ad754, 0205300) from the @freddiemixell's fork.
The tests ensure the feature works as designed and the documentation provides clear guidance for developers on how to use translatable_blocks to control which StructBlock sub-fields are translatable, addressing the long-standing request in issue #307.
Closes #752.
CC: @freddiemixell (?) @zerolab for review. Appreciate any feedback!