search

wagtail / wagtail-localize

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

TranslationSource uniqueness issue #662

Open krieghan opened 1 year ago

krieghan commented 1 year ago

We noted a traceback recently when editing pages. We're using:

wagtail==3.0.1 wagtail-localize==1.2.1

Traceback ``` Traceback (most recent call last): File "/usr/local/lib/python3.8/site-packages/django/core/handlers/exception.py", line 47, in inner response = get_response(request) File "/usr/local/lib/python3.8/site-packages/django/core/handlers/base.py", line 181, in _get_response response = wrapped_callback(request, *callback_args, **callback_kwargs) File "/usr/local/lib/python3.8/site-packages/sentry_sdk/integrations/django/views.py", line 67, in sentry_wrapped_callback return callback(request, *args, **kwargs) File "/usr/local/lib/python3.8/site-packages/django/views/decorators/cache.py", line 44, in _wrapped_view_func response = view_func(request, *args, **kwargs) File "/usr/local/lib/python3.8/site-packages/wagtail/admin/urls/__init__.py", line 161, in wrapper return view_func(request, *args, **kwargs) File "/usr/local/lib/python3.8/site-packages/wagtail/admin/auth.py", line 182, in decorated_view response = view_func(request, *args, **kwargs) File "/usr/local/lib/python3.8/site-packages/django/views/generic/base.py", line 70, in view return self.dispatch(request, *args, **kwargs) File "/usr/local/lib/python3.8/site-packages/wagtail/admin/views/pages/edit.py", line 341, in dispatch response = self.run_hook("before_edit_page", self.request, self.page) File "/usr/local/lib/python3.8/site-packages/wagtail/admin/views/generic/hooks.py", line 16, in run_hook result = fn(*args, **kwargs) File "/usr/local/lib/python3.8/site-packages/wagtail_localize/wagtail_hooks.py", line 259, in before_edit_page return edit_translation.edit_translation(request, translation, page) File "/usr/local/lib/python3.8/site-packages/wagtail_localize/views/edit_translation.py", line 912, in edit_translation locale_id=TranslationSource.objects.get( File "/usr/local/lib/python3.8/site-packages/django/db/models/manager.py", line 85, in manager_method return getattr(self.get_queryset(), name)(*args, **kwargs) File "/usr/local/lib/python3.8/site-packages/django/db/models/query.py", line 439, in get raise self.model.MultipleObjectsReturned( wagtail_localize.models.TranslationSource.MultipleObjectsReturned: get() returned more than one TranslationSource -- it returned 2! ```

It was a minor emergency, and we had to delete a lot of the data in question, and we're not entirely sure how the data got into this state. We've been on the lookout for recurrences, but so far we haven't been able to reproduce it, and I haven't found any similar issues logged here. My question for your project is about potential uniqueness constraints. The statement which causes the exception is this:

        add_convert_to_alias_url = (
            Page.objects.filter(
                translation_key=instance.translation_key,
                locale_id=TranslationSource.objects.get(
                    object_id=instance.translation_key,
                    specific_content_type=instance.content_type_id,
                    translations__target_locale=instance.locale,
                ).locale_id,
            )
            .exclude(pk=instance.pk)
            .exists()
        )

The call to get a TranslationSource is assuming that there will be only one for each object_id, specific_content_type, and target_locale. Is enforcing uniqueness on this model possible, or desirable?

zerolab commented 1 year ago

I am not sure you ended up with multiple TranslationSources for the given page. Regardless, we most certainly should be a bit more defensive around that area

https://github.com/wagtail/wagtail-localize/blob/876dd6e559ef7bcf1ce533793ab80b3119c1392a/wagtail_localize/models.py#L331 ensures uniqueness based on object and locale. I take it the combination of content type and translations and perhaps a locale following another...

Regardless, the first thing would be to add TranslationSource.MultipleObjectsReturned to https://github.com/wagtail/wagtail-localize/blob/876dd6e559ef7bcf1ce533793ab80b3119c1392a/wagtail_localize/views/edit_translation.py#L969, but the important bit would be to have a reproducible test case and understand whether there is more impact in other places

RayyanRiaz commented 1 year ago

Hi, I recently started playing around with wagtail-localize, and I can reproduce this with the following dependencies:

    Django==4.0.3
    wagtail==2.16.1
    wagtail-localize==1.1.1

if we have multiple languages, we have to use a single source page for translation into the other languages. e.g., If we translate en into de and then try to translate de version into fr, we will get an error.

EDIT:

I found that my issue was related to this PR. So, updating to 1.3.3 already solved it for me. Not sure if the current issue is also related to that

zerolab commented 1 year ago

Related discussion, with additional info - https://github.com/wagtail/wagtail-localize/discussions/713

kmtracey commented 7 months ago

For whatever it's worth, we've recently run into this as well, though based on when the TranslationSource and Translation instances were created it's been a long time in the brewing. The site in question is pretty much static once a locale's page tree is populated, but something caused a desire to edit Australian English pages recently and the edit pages raised this error. We wrote a command to find and show some information about pages in a bad state and this is what it reported:

Found 6 page(s) with duplicate translations
Locale English (Australia) page title Home (en-au)
    Translation source locale English (United States) created 2022-02-21 16:23:09.908579+00:00; translation created at 2022-02-21 16:23:10.284596+00:00, translation.enabled=False
    (KEEP) Translation source locale Danish (Denmark) created 2022-06-14 17:41:04.588877+00:00; translation created at 2022-06-14 17:41:05.071662+00:00, translation.enabled=False
Locale English (Australia) page title About <redacted>
    Translation source locale English (United States) created 2022-02-21 16:23:11.074708+00:00; translation created at 2022-02-21 16:23:11.671899+00:00, translation.enabled=False
    (KEEP) Translation source locale Danish (Denmark) created 2022-06-14 17:41:06.566145+00:00; translation created at 2022-06-14 17:41:07.170768+00:00, translation.enabled=False
Locale English (Australia) page title About <redacted>
    Translation source locale English (United States) created 2022-02-21 16:23:12.385411+00:00; translation created at 2022-02-21 16:23:12.674458+00:00, translation.enabled=False
    (KEEP) Translation source locale Danish (Denmark) created 2022-06-14 17:41:08.592466+00:00; translation created at 2022-06-14 17:41:08.993517+00:00, translation.enabled=False
Locale English (Australia) page title <redacted>
    Translation source locale English (United States) created 2022-02-21 16:23:13.374364+00:00; translation created at 2022-02-21 16:23:13.786154+00:00, translation.enabled=False
    (KEEP) Translation source locale Danish (Denmark) created 2022-06-14 17:41:10.569988+00:00; translation created at 2022-06-14 17:41:11.067056+00:00, translation.enabled=False
Locale English (Australia) page title Terms of Use
    Translation source locale English (United States) created 2022-02-21 16:23:14.563948+00:00; translation created at 2022-02-21 16:23:14.799835+00:00, translation.enabled=False 
    (KEEP) Translation source locale Danish (Denmark) created 2022-06-14 17:41:12.479573+00:00; translation created at 2022-06-14 17:41:12.794957+00:00, translation.enabled=False
Locale Portuguese (Brazil) page title <redacted>
    (KEEP) Translation source locale English (United States) created 2022-02-21 16:23:13.374364+00:00; translation created at 2023-07-27 16:21:35.843032+00:00, translation.enabled=False
    Translation source locale English (Australia) created 2023-05-10 13:55:42.061246+00:00; translation created at 2023-07-13 15:04:53.640074+00:00, translation.enabled=False

The site only has 5 pages I think, so it seems the entire Australian tree was in troubled state while just one of the Brazilian Portuguese pages had gotten into this state (the dates on that one are very different too) -- it seems there may be a couple of different paths to getting into this state.

However, we have not been able to figure out how to create the duplicative translations that are the root cause of the 500 error and given how long ago they were all created it's a bit of mystery to us how it happened. It seems all pages have been converted to non-sync'd translations and are directly editable now, not sure if that plays into this at all. Deleting the older (and disabled) translation instances allows for edit on the pages that were showing server errors.