On Sun, Jun 8, 2014 at 6:15 PM, Mathias Ettinger <[email protected] > wrote:
> You’re pretty right, some design choices sound really bad when put like > this. I’ll move the admin classes, the code_list and the frontend language > selector somewhere else. > Not bad per se, it's common for these things to involve some iterations :-) > As for the TRANSLATED setting, I don’t recall exactly why I tied it with > USE_I18N. If I’m correct, it is because django-modeltranslation deactivates > itself when USE_I18N is False. So we can’t relly solely on > USE_MODELTRANSLATION to know when translations are activated. I’ll check > that by tomorow and let you know. But if it is the case, what is your > suggested approach ? Force the USE_I18N to True if USE_MODELTRANSLATION is > True ? Tighten TRANSLATED into settings instead of a global var ? > So if modeltranslations needs USE_I18N then that's fine - let's just comment that in the code where we deal with it. I guess what I was getting at was that we should try and have this configurable via a single point, namely USE_MODELTRANSLATION - we can handle all the rest in set_dynamic_settings. There we can check if USE_MODELTRANSLATION is True and if so, also check that we can import modeltranslations, and then with that in place we can do the rest - set USE_I18N to True, and add modeltranslations to INSTALLED_APPS if missing. After that we can reliably use mezzanine.conf.settings.USE_MODELTRANSLATION *everywhere* to control integration (such as which admin class to use, what appears on the front-end, etc). > > > Le samedi 7 juin 2014 12:09:02 UTC+2, Stephen McDonald a écrit : >> >> Ok I spent a bit of time going over the pull request and testing it, it's >> really good. >> >> I think we can tighten things up a little, for example we have `class >> SomeAdmin(TRANSLATED and TranslationAdmin or admin.ModelAdmin)` all through >> each of the admin modules. I think we could have a single BaseAdmin class >> with this logic that everything subclasses, rather than having conditions >> all throughout the code base. With that in place I figure it'd be pretty >> easy for us to modify that base class to include a snippet of jQuery with a >> single toggle (eg per screen, not per field, as Ed suggested) that >> shows/hides all the language fields. >> >> Also confused about the end of mezzanine/conf/__init__: >> >> TRANSLATED = settings.USE_MODELTRANSLATION and settings.USE_I18N >> CODE_LIST = [lang[0] for lang in settings.LANGUAGES] >> >> This isn't where settings should simply be dumped. CODE_LIST could >> possibly be a function in mezzanine.utils.translation. Does Django not have >> this already? >> >> More importantly I don't understand the relationship between >> USE_MODELTRANSLATION and USE_I18N, and why these get combined into another >> setting (which isn't consistently used throughout the PR, in same cases >> this combo of settings get checked separately again). Can't everything just >> check the USE_MODELTRANSLATION settings? If USE_I18N is needed for this to >> work, then that's what's set_dynamic_settings is for - fixing up missing >> things that the developer has intended to have working by providing a >> single point of configuration that hides away all the various bits that >> need configuring. In set_dynamic_settings we should simply check for >> USE_MODELTRANSLATION and if True, add modeltranslation app if missing, and >> if USE_I18N actually needs to be True for modeltranslation to work, set it >> to True as well. modeltranslation shouldn't be in INSTALLED_APPS by default >> either. >> >> Lastly we probably don't want the language chooser for the front-end site >> in the top nav, there's really no room there. Perhaps it could sit on the >> right-hand panel at the top somewhere along with the login bits, >> conditionally, if USE_MODELTRANSLATION is True. All of this stuff should be >> turned off by default which means USE_MODELTRANSLATION defaults to False >> and *everything* else stems from that. >> >> If I've stupidly overlooked anything please let me know. This looks real >> promising and I'm hoping this along with django-rest-framework integration >> will make a compelling offering for the next major release 3.2. I'm going >> to push 3.1.5 shortly with some bug fixes, and hopefully that'll be the >> last 3.1.x release. >> >> Thanks Mathias and Ed for working this all out, it looks really really >> good. >> >> >> >> >> >> >> >> On Sat, Jun 7, 2014 at 2:31 PM, Stephen McDonald <[email protected]> wrote: >> >>> Real odd, works fine for me now and no matter what I try I can't >>> reproduce the error I had. >>> >>> >>> On Sat, Jun 7, 2014 at 2:09 PM, Eduardo Rivas <[email protected]> >>> wrote: >>> >>>> Odd, I just created a project without issue. This is my venv: >>>> >>>> Django==1.6.5 >>>> Mezzanine==3.1.3 #From Mathias' branch >>>> Pillow==2.4.0 >>>> South==0.8.4 >>>> argparse==1.2.1 >>>> bleach==1.4 >>>> django-debug-toolbar==1.2.1 >>>> django-modeltranslation==0.7.3 >>>> filebrowser-safe==0.3.3 >>>> future==0.9.0 >>>> grappelli-safe==0.3.10 >>>> html5lib==0.999 >>>> oauthlib==0.6.1 >>>> psycopg2==2.5.3 >>>> pytz==2014.4 >>>> requests==2.3.0 >>>> requests-oauthlib==0.4.0 >>>> six==1.6.1 >>>> sqlparse==0.1.11 >>>> tzlocal==1.0 >>>> wsgiref==0.1.2 >>>> >>>> I ran mezzanine-project and createdb --noinput. Have you done any other >>>> changes to settings? >>>> >>>> -- >>>> You received this message because you are subscribed to the Google >>>> Groups "Mezzanine Users" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to [email protected]. >>>> >>>> For more options, visit https://groups.google.com/d/optout. >>>> >>> >>> >>> >>> -- >>> Stephen McDonald >>> http://jupo.org >>> >> >> >> >> -- >> Stephen McDonald >> http://jupo.org >> > -- > You received this message because you are subscribed to the Google Groups > "Mezzanine Users" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/d/optout. > -- Stephen McDonald http://jupo.org -- You received this message because you are subscribed to the Google Groups "Mezzanine Users" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
