wagtail / wagtail-localize

Translation plugin for Wagtail CMS
https://wagtail-localize.org/
Other
222 stars 84 forks source link

Create a translated page Error: `string` must be either a `StringValue` or a `str`. Got `NoneType` #690

Open PeteCoward opened 1 year ago

PeteCoward commented 1 year ago

If you have a page type with a StreamField, which uses a StructBlock, and you add a new TextBlock(required=False) field to that struct block, and run all migrations, the resultant JSON in the db will contain a None value. StreamFieldSegmentExtractor fails to handle that None value, and an exception is thrown.

I got around this by a custom stream field data migration forceably setting that field on all instances of the StructBlock, but it's a bit painful and is lying in wait for my team and any others who might do this in future.

Some simple changes to StringSegmentValue could fix this I think, but I ended up going with the data migration. I did fork initially, but pip installing from my fork failed due to the javascript compilation not being run. Then I looked for another solution and went with data migrations.

PeteCoward commented 1 year ago

If I get the time I may try to set up a dev environment, but adding a CONTRIBUTING.md would really help!

zerolab commented 1 year ago

Thanks @PeteCoward We have https://github.com/wagtail/wagtail-localize#contributing, but a separate CONTRIBUTING.md sounds like a good idea

PeteCoward commented 1 year ago

Some simple changes to StringSegmentValue could fix this I think

@zerolab I suspect I was wrong here, from what I can understand so far StringSegmentValue is used both for:

  1. portions of RichTextField and RichTextBlock content
  2. individual Text(like)Fields and Text(Like)Block content

The behaviour of suppressing empty sections of RichText makes sense now to me, but it seems to make sense to me to make sure that empty values are available for translation if they are top level fields on models or blocks.

I wanted to check my understanding with you not being familiar with the codebase before diving into some kind of solution which might involve a refactor to StringSegmentValue and FieldStringSegmentValue ( or the reverse naming RichStringSegmentValue and StringSegmentValue ) which would allow targeting behaviour at the non rich text content.

zerolab commented 1 year ago

@PeteCoward I need to make that dive myself as it has been a while since I looked and the two segments.

It is worth noting they have been added prior to Wagtail requiring use_json_field=True on StreamFields

PeteCoward commented 1 year ago

I figure you might still want to support use_json_field=False for a while though?

zerolab commented 1 year ago

Most definitely. Wagtail 4.1, which is an LTS still supports use_json_field=False with plenty of warnings.

tognee commented 3 weeks ago

This issue is still present in the latest version
Re-publishing the page from the editor before creating the translated page solves the issue as it replaces all the None fields with an empty string