idlesign / django-siteprefs

Reusable app for Django introducing site preferences system
https://github.com/idlesign/django-siteprefs
BSD 3-Clause "New" or "Revised" License
17 stars 6 forks source link

PrefProxy fails without calling get_value() #5

Closed electroniceagle closed 10 years ago

electroniceagle commented 10 years ago

This is probably my own misunderstanding, but the setting returns a string "SHOP_DASHBOARD_URL = /dashboard", rather than "/dashboard". Our workaround is to call .get_value() on the PrefProxy object, but not sure this is expected behavior.

We have this settings.py:

from django.conf import settings
from django.core.urlresolvers import reverse_lazy
SHOP_DASHBOARD_URL = getattr(
    settings, 'SHOP_DASHBOARD_URL', reverse_lazy('session_list'))
try:
    from siteprefs.toolbox import (
        patch_locals, register_prefs, pref, pref_group)
    patch_locals()
    register_prefs(
        pref(SHOP_DASHBOARD_URL, static=False,
             verbose_name='Dashboard URL',
             help_text="Dashboard URL for session products.  "
             "Default is '/sessions'."),
    )
except ImportError:
    pass

and then in views.py:

from .settings import SHOP_DASHBOARD_URL
...
import pdb
pdb.set_trace()
(Pdb) p SHOP_DASHBOARD_URL
<siteprefs.utils.PrefProxy object at 0x102318c90>
(Pdb) p str(SHOP_DASHBOARD_URL)
'SHOP_DASHBOARD_URL = /dashboard'
(Pdb)
idlesign commented 10 years ago

You're casting SHOP_DASHBOARD_URL into a string for pdb, so PrefProxy triggers str(), giving SHOP_DASHBOARD_URL = /dashboard as expected. You'd better use call(), but direct get_value() will do as well.

Nevertheless we can upgrade str machinery: thus the current str will go into repr, so a new str might provide some other logic.

electroniceagle commented 10 years ago

Thanks for the help! Maybe my pdb example was misleading. What we hit was:

return HttpResponseRedirect(SHOP_DASHBOARD_URL)

redirected to this:

http://localhost:8000/en/shop/checkout/SHOP_DASHBOARD_URL%20=%20/sessions

Using the settings.py taken from your doc example seems inconsistent if that is the intended behavior. Without siteprefs you get 'bar', with siteprefs you get 'foo = bar'.

idlesign commented 10 years ago

The thing is that you're using reverse_lazy() that returns a proxy itself - it evaluates a result only as required. HttpResponseRedirect expects a string so it casts a proxy into a string.

Examples from the docs use plain objects not proxies and are consistent from that point of view.

Now I'll do some changes so taht you could verify they work.

idlesign commented 10 years ago

You can try now.

electroniceagle commented 10 years ago

Thanks! I understand now. Completely forgot about the reverse_lazy().

idlesign commented 10 years ago

Is https://github.com/idlesign/django-siteprefs/commit/51623e2618830e25e42d333a487571768f10a92b of any help?

electroniceagle commented 10 years ago

That worked, thanks.

On May 9, 2014, at 11:15 AM, Igor Starikov notifications@github.com wrote:

Is 51623e2 of any help?

— Reply to this email directly or view it on GitHub.