search

wagtail / wagtailtrans

A Wagtail add-on for supporting multilingual sites
http://wagtailtrans.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
104 stars 60 forks source link

Fix #132 Default language no longer in other_languages #133

Closed rasca closed 6 years ago

rasca commented 6 years ago

I've modified a test that fails without this changes.

Also I fixed Django version to 2.0 because wagtail doesn't support 2.1 as of today.

Henk-JanVanHasselaar commented 6 years ago

@rasca first off, thank you for taking the time write a fix for this!

If I understand you correctly there is a problem when you have LANGUAGES_PER_SITE enabled and try to save a SiteLanguage setting where the default language is also selected in the other_languages field. This then causes a recursion error because you keep signaling a save for the default language.

If i'm correct isn't the best solution to catch this in form validation instead of another signal? because then you'll catch it before it even becomes a problem.

mikedingjan commented 6 years ago

I agree with @Henk-JanVanHasselaar on this, to validate this in a form instead of another signal, I've created a PR #141 which implements this. Didn't check this with all the different configurations yet.

@rasca can you check if this solves the problem for you?

mikedingjan commented 6 years ago

I'm closing this one if favor of #141