search

wagtail / wagtail-localize

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

Support for translating snippets in draft mode #666

Closed hpoul closed 1 month ago

hpoul commented 1 year ago

Since Wagtail 4.0 snippets can be saved as draft by using DraftStateMixin. When translating a snippet which is still in draft mode the following exception is raised:

  File "/Users/herbert/Library/Caches/pypoetry/virtualenvs/sportpark-backend-ztpaQDXI-py3.10/lib/python3.10/site-packages/wagtail_localize/operations.py", line 90, in translate_object
    translator.create_translations(instance)
  File "/Users/herbert/Library/Caches/pypoetry/virtualenvs/sportpark-backend-ztpaQDXI-py3.10/lib/python3.10/site-packages/wagtail_localize/operations.py", line 79, in create_translations
    translation.save_target(user=self.user, publish=publish)
  File "/Users/herbert/Library/Caches/pypoetry/virtualenvs/sportpark-backend-ztpaQDXI-py3.10/lib/python3.10/site-packages/wagtail_localize/models.py", line 1334, in save_target
    self.source.create_or_update_translation(
  File "/Users/herbert/Library/Caches/pypoetry/virtualenvs/sportpark-backend-ztpaQDXI-py3.10/lib/python3.10/site-packages/wagtail_localize/models.py", line 742, in create_or_update_translation
    raise CannotSaveDraftError
wagtail_localize.models.CannotSaveDraftError

I guess this check was implemented before snippets could be saved as draft, and can be simply removed? 🤔 or using isinstance(original, DraftStateMixin) instead of isinstance(original, Page)?

https://github.com/wagtail/wagtail-localize/blame/abf2853ad7660dde7b9010de36187e618d4d9184/wagtail_localize/models.py#L740-L742

zerolab commented 1 year ago

@hpoul the check was indeed added way before Wagtail 4.0 considering we're still supporting 2.15, there needs to be a conditional check and the appropriate logic. isinstance(original, DraftStateMixin) feels like the right move for Wagtail 4.0+. Just need to check other places where we check whether it is a Page (e.g. there's logic around publishing when syncing translations)

A PR would be most welcome

hpoul commented 1 year ago

I'm not sure if I'll be able to provide a PR, for full support for drafts and versioning it would probably require some more changes, like the permission checks in edit_translation... combined with making it backward compatibility and adding tests, this is probably not a trivial fix after all.. I think i'll just make my model a Page 😂 it feels like there is no chance this will be the last problem i'd encounter when using snippets.. (I'm basically using wagtail as a headless cms for an app, so it would have been nice to get rid of all those web-related properties, but it seems it's more stable to just go with it)

zerolab commented 1 month ago

Should be fixed by #751 / v1.9. Please re-open if still an issue