Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for translating snippets in draft mode #666

Closed
hpoul opened this issue Dec 3, 2022 · 3 comments
Closed

Support for translating snippets in draft mode #666

hpoul opened this issue Dec 3, 2022 · 3 comments
Labels
help wanted Extra attention is needed type: DX Developer eXperience

Comments

@hpoul
Copy link
Contributor

hpoul commented Dec 3, 2022

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
Copy link
Collaborator

zerolab commented Dec 3, 2022

@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
Copy link
Contributor Author

hpoul commented Dec 10, 2022

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 zerolab added help wanted Extra attention is needed type: DX Developer eXperience labels Oct 6, 2023
@zerolab
Copy link
Collaborator

zerolab commented Aug 3, 2024

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

@zerolab zerolab closed this as completed Aug 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed type: DX Developer eXperience
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants