Allow defining translatable_fields/override_translatable_fields on StructBlock #752

freddiemixell · 2023-12-18T02:09:34Z

This PR is attempting to allow users to mark certain sub-blocks of a struct block to be ignored like UrlBlock.

Example use case:
blocks.py

class RichTextBlock(blocks.StructBlock):
    """Rich text block."""

    rich = blocks.RichTextBlock(
        required=True,
        help_text="Add a rich text context",
        features=['h2', 'h3', 'h4', 'h5', 'h6', 'bold', 'italic', 'link', 'ol', 'ul', 'hr', 'superscript', 'subscript', 'strikethrough', 'blockquote', 'code'],
    )
   
class CustomImageBlock(blocks.StructBlock):
    """Banner image block."""

    image = ImageChooserBlock(required=True, help_text="Add a banner image.")
    description = blocks.TextBlock(required=False, help_text="Add a description for the image.")
    
    override_translatable_blocks = [
        "description"
    ]

class YouTubeBlock(blocks.StructBlock):
    """YT Video block."""

    video_id = blocks.CharBlock(
        required=True,
        help_text="Add a YouTube video ID"
    )

    override_translatable_blocks = []
    
class CodeBlock(blocks.StructBlock):
    """Code block."""

    code = blocks.TextBlock(required=True, help_text="Add your code snippet here.")
    language = blocks.ChoiceBlock(choices=[
        ('python', 'Python'),
        ('javascript', 'JavaScript'),
        ('java', 'Java'),
        ('c', 'C'),
    ], required=True, help_text="Select the language of your code snippet.")

    override_translatable_blocks = []

In the above example the first block doesn't define the property so it goes through the code the same as before.

The second block only includes the description field, which technically doesn't matter because the image would be skipped regardless. However, I added it in to illustrate how you could exclude fields by simply not adding them to the list.

The last 2 examples we add the property but with an empty list. That will prevent any blocks from this struct block from being extracted.

Notes:
Closes #307

codecov-commenter · 2023-12-18T09:29:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (a3efa8f) 92.60% compared to head (5fb32e5) 92.65%.
Report is 13 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #752      +/-   ##
==========================================
+ Coverage   92.60%   92.65%   +0.05%     
==========================================
  Files          46       47       +1     
  Lines        4017     4072      +55     
  Branches      598      603       +5     
==========================================
+ Hits         3720     3773      +53     
- Misses        175      176       +1     
- Partials      122      123       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

zerolab · 2023-12-20T12:26:39Z

Thank you @freddiemixell. Could you please add some tests?

freddiemixell · 2023-12-20T15:07:11Z

No problem! I begin working on them already but alas the work week came and stole me away lol

I will have some time starting this weekend and I'm off all next week so I will wrap this up then if I don't find during the week.

freddiemixell · 2023-12-24T14:37:32Z

@zerolab I figured out some intial tests. Let me know if you'd like more or any changes.

Happy holidays! ☃️

zerolab

This is a solid start @freddiemixell

Can you update

wagtail-localize/docs/how-to/field-configuration.md

Lines 3 to 5 in 92c18ab

 !!! attention 

 It is not currently possible to configure `StructBlock` translatable fields. See [Issue #307](https://github.com/wagtail/wagtail-localize/issues/307) for more details.

and remove the notices, as well as add a section at the end on how to use this?

Aside from that, I keep going back to the fact that for regular models we allow defining whether a field is translatable or is synchronised, and that we need to support this for StructBlocks.
For example, in your CustomImageBlock we want the description to be translatable, but the image to be synchronised. I am making an assumption here as I am not fully geared to test things locally ('tis Christmas after all), but what happens in that case?

wagtail_localize/segments/extract.py

freddiemixell · 2024-01-04T15:22:04Z

This is a solid start @freddiemixell

Can you update

wagtail-localize/docs/how-to/field-configuration.md

Lines 3 to 5 in 92c18ab

!!! attention

It is not currently possible to configure `StructBlock` translatable fields. See [Issue #307](https://github.com/wagtail/wagtail-localize/issues/307) for more details.

and remove the notices, as well as add a section at the end on how to use this?
Aside from that, I keep going back to the fact that for regular models we allow defining whether a field is translatable or is synchronised, and that we need to support this for StructBlocks. For example, in your CustomImageBlock we want the description to be translatable, but the image to be synchronised. I am making an assumption here as I am not fully geared to test things locally ('tis Christmas after all), but what happens in that case?

Hey @zerolab,

Thanks for checking this out on Christmas! (Tis Christmas made me lol)

I see what you're saying about the image block and synchronization. I'll investigate if this will cause any issues with synchronized fields today.

If you have any specific tests you're thinking of let me know and I'll try to hook it up, thanks again!

freddiemixell · 2024-01-04T16:10:49Z

How I'm Testing

Go to english source page.
Click "Sync translated pages" but don't publish.
Go to french version of the page to inspect what is in the UI.
Publish the french version and screen shot the frontend results.
Go back to the english version and screenshot to make sure nothing is changing.

How the source test post is structured:

Scenario 1: Testing ignoring the description struct field

StructBlock:

image = ImageChooserBlock
description = CharBlock
translatable_blocks = []

Expected:

image block still available to override
description block not showing

Result:

image block still available to override
description block not showing

English Page

French Page

Scenario 2: Testing explicitly telling it description should be translated

StructBlock:

image = ImageChooserBlock
description = CharBlock
translatable_blocks = ['description']

Expected:

image block still available to override
description available to translate
note: this should be the same as doing nothing, scenario 3.

Result:

image block still available to override
description available to translate

English Page

French Page

Scenario 3: Testing default/backwards compatibility

StructBlock:

image = ImageChooserBlock
description = CharBlock

Expected:

image block still available to override
description available to translate

Result:

image block still available to override
description available to translate

English Page

French Page

freddiemixell · 2024-01-04T16:14:16Z

I'll add that I'm not sure why exactly this is working with the images. That is what I'm going to work on figuring out now so I could really get behind this or find any flaws.

freddiemixell · 2024-01-04T16:20:34Z

Issue:

When overriding the image of the youtube button in the french version with a picture of the Eiffel Tower I still see the Youtube button.

I believe this means it's breaking the overridable segments somehow.

Update:

I reverted this change a second time, reset everything and now I do see the image block going through so this seems like a problem with this PR still.

Whats actually happening:
After testing this a bit further I'm seeing that I was mistaken before. I was getting confused because I had an image field on my blog model and I was mistaking that for the image block. I changed it to another image and that revealed the issue.

translatable_blocks = ['description']

This prevents the images from being overridden, which is sort of enforced by this configuration if you wanted to prevent images from being changed for some reason.

translatable_blocks = ['image']

However, this configuration produces a different scenario where you can keep the same text but allow overriding the image. When I did this I was able to override the image successfully in the French version. Which means this isn't technically a bug but does allow for a little more control over nested images if you need it.

zerolab · 2024-01-04T19:11:18Z

docs/how-to/field-configuration.md

+
+## Specifying Translatable Fields within a StructBlock
+
+**By default, all sub-fields within a StructBlock are included for translation.** However, there may be instances where certain fields within the StructBlock should be excluded from translation. To facilitate this, we've introduced the **`translatable_blocks`** parameter.


nitpick: no need to wrap inline code ** (bold)

zerolab · 2024-01-04T19:12:31Z

docs/how-to/field-configuration.md

+
+## Specifying Translatable Fields within a StructBlock
+
+**By default, all sub-fields within a StructBlock are included for translation.** However, there may be instances where certain fields within the StructBlock should be excluded from translation. To facilitate this, we've introduced the **`translatable_blocks`** parameter.


suggestion: wrap StructBlock, CharBlock, TextBlock and YouTubeBlock in backticks (so they are treated as inline code)

zerolab · 2024-01-04T19:14:47Z

docs/how-to/field-configuration.md

+
+##### Managing Images and Overrideable Segments within a StructBlock
+
+When dealing with overrideable segments such as images within a StructBlock, it's important to note that ignoring these segments could result in losing the ability to use a different image for different languages. If you want to maintain the ability to override an image, include it in the list of translatable_blocks to preserve the default behavior.


suggestion:

Suggested change

When dealing with overrideable segments such as images within a StructBlock, it's important to note that ignoring these segments could result in losing the ability to use a different image for different languages. If you want to maintain the ability to override an image, include it in the list of translatable_blocks to preserve the default behavior.

It is important to note that ignoring overrideable segments such as images within a `StructBlock` could result in losing the ability to use a different image for different languages. If you want to maintain the ability to override an image, include it in the list of `translatable_blocks` to preserve the default behavior.

zerolab · 2024-01-04T19:16:16Z

docs/how-to/field-configuration.md

+ translatable_blocks = ["caption", "image"]
+```
+
+In this example, the image is still overrideable, but the address, which is unique to this location, is locked in by translatable_blocks. This allows you to maintain the flexibility of using different images for different languages, while ensuring that certain unique information remains consistent across all translations.


Suggested change

In this example, the image is still overrideable, but the address, which is unique to this location, is locked in by translatable_blocks. This allows you to maintain the flexibility of using different images for different languages, while ensuring that certain unique information remains consistent across all translations.

In this example, the image is still overrideable. The address, nique to this location, is locked in by `translatable_blocks`. This allows you to maintain the flexibility of using different images for different languages, while ensuring that certain unique information remains consistent across all translations.

zerolab · 2024-01-04T19:17:13Z

@freddiemixell the code tests should pretty much follow what you test as a user, manually

zerolab · 2024-02-06T21:07:10Z

Hey @freddiemixell do you need any help / pointers on tests?

allow list of include blocks to override

bf11b23

zerolab added status: Needs tests status: Needs review labels Dec 18, 2023

Frederick Mixell added 2 commits December 24, 2023 09:28

match other override syntax

00644b1

adding test cases

ecadd52

zerolab reviewed Dec 25, 2023

View reviewed changes

wagtail_localize/segments/extract.py Outdated Show resolved Hide resolved

Frederick Mixell added 2 commits January 4, 2024 12:50

revert naming

dea103e

adding docs

5fb32e5

zerolab reviewed Jan 4, 2024

View reviewed changes

zerolab removed the status: Needs review label Mar 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow defining translatable_fields/override_translatable_fields on StructBlock #752

Allow defining translatable_fields/override_translatable_fields on StructBlock #752

freddiemixell commented Dec 18, 2023 •

edited

codecov-commenter commented Dec 18, 2023 •

edited

zerolab commented Dec 20, 2023

freddiemixell commented Dec 20, 2023

freddiemixell commented Dec 24, 2023

zerolab left a comment

freddiemixell commented Jan 4, 2024

freddiemixell commented Jan 4, 2024

freddiemixell commented Jan 4, 2024

freddiemixell commented Jan 4, 2024 •

edited

zerolab Jan 4, 2024

zerolab Jan 4, 2024

zerolab Jan 4, 2024

zerolab Jan 4, 2024

zerolab commented Jan 4, 2024

zerolab commented Feb 6, 2024

	!!! attention

	It is not currently possible to configure `StructBlock` translatable fields. See [Issue #307](https://github.com/wagtail/wagtail-localize/issues/307) for more details.


		## Specifying Translatable Fields within a StructBlock

		By default, all sub-fields within a StructBlock are included for translation. However, there may be instances where certain fields within the StructBlock should be excluded from translation. To facilitate this, we've introduced the `translatable_blocks` parameter.


		##### Managing Images and Overrideable Segments within a StructBlock

		When dealing with overrideable segments such as images within a StructBlock, it's important to note that ignoring these segments could result in losing the ability to use a different image for different languages. If you want to maintain the ability to override an image, include it in the list of translatable_blocks to preserve the default behavior.

	When dealing with overrideable segments such as images within a StructBlock, it's important to note that ignoring these segments could result in losing the ability to use a different image for different languages. If you want to maintain the ability to override an image, include it in the list of translatable_blocks to preserve the default behavior.
	It is important to note that ignoring overrideable segments such as images within a `StructBlock` could result in losing the ability to use a different image for different languages. If you want to maintain the ability to override an image, include it in the list of `translatable_blocks` to preserve the default behavior.

	In this example, the image is still overrideable, but the address, which is unique to this location, is locked in by translatable_blocks. This allows you to maintain the flexibility of using different images for different languages, while ensuring that certain unique information remains consistent across all translations.
	In this example, the image is still overrideable. The address, nique to this location, is locked in by `translatable_blocks`. This allows you to maintain the flexibility of using different images for different languages, while ensuring that certain unique information remains consistent across all translations.

Allow defining translatable_fields/override_translatable_fields on StructBlock #752

Are you sure you want to change the base?

Allow defining translatable_fields/override_translatable_fields on StructBlock #752

Conversation

freddiemixell commented Dec 18, 2023 • edited

codecov-commenter commented Dec 18, 2023 • edited

Codecov Report

zerolab commented Dec 20, 2023

freddiemixell commented Dec 20, 2023

freddiemixell commented Dec 24, 2023

zerolab left a comment

Choose a reason for hiding this comment

freddiemixell commented Jan 4, 2024

freddiemixell commented Jan 4, 2024

How I'm Testing

How the source test post is structured:

Scenario 1: Testing ignoring the description struct field

Scenario 2: Testing explicitly telling it description should be translated

Scenario 3: Testing default/backwards compatibility

freddiemixell commented Jan 4, 2024

freddiemixell commented Jan 4, 2024 • edited

zerolab Jan 4, 2024

Choose a reason for hiding this comment

zerolab Jan 4, 2024

Choose a reason for hiding this comment

zerolab Jan 4, 2024

Choose a reason for hiding this comment

zerolab Jan 4, 2024

Choose a reason for hiding this comment

zerolab commented Jan 4, 2024

zerolab commented Feb 6, 2024

freddiemixell commented Dec 18, 2023 •

edited

codecov-commenter commented Dec 18, 2023 •

edited

freddiemixell commented Jan 4, 2024 •

edited