django-user-roles icon indicating copy to clipboard operation
django-user-roles copied to clipboard

fixing the migration to go with the rest of the 1.5 user model changes

Open tijs opened this issue 12 years ago • 9 comments

I would say that this at least fixes it for now so people downloading it will have a working version. The other use cases could then be looked at at a case by case basis perhaps?

tijs avatar Jul 31 '13 13:07 tijs

Would the following also work?...

user_orm_label = getattr(settings, 'AUTH_USER_MODEL', 'auth.User')
model_orm_label = user_orm_label.lower()

If so (?) it'd be cleaner, and using 'AUTH_USER_MODEL' is preferable to get_user_model as it doesn't require importing the models module, which can cause issues in some cases dues to unexpected cyclical deps or suchlike.

lovelydinosaur avatar Jul 31 '13 14:07 lovelydinosaur

The get_user_model bit makes this work too though:

'Meta': {'object_name': User.__name__, 'db_table': "'%s'" % User._meta.db_table},

Maybe there's a cleaner way to solve that as well?

tijs avatar Jul 31 '13 14:07 tijs

Oh yikes. Yeah maybe what you have is best. Not super-comfortable with the whole crazy migration dance, but might be the only approach. Anyone seen an other projects dealing with this, and happy that this approach is sensible?

lovelydinosaur avatar Jul 31 '13 14:07 lovelydinosaur

My current approach is something i picked from other projects, i don't think there is a gold standard for this yet. would be nice if there was a 1.5 migration best practice somewhere. Maybe we should ask @pydanny? ;)

tijs avatar Jul 31 '13 14:07 tijs

Nothing so far. Come up with something, justify it nicely, and maybe it will become the standard. :100:

pydanny avatar Jul 31 '13 14:07 pydanny

@tijs - I'd be happier with something like this... https://github.com/caffeinehit/django-oauth2-provider/pull/18/files

Seems simpler / safer to me. Doesn't cover the case of using a custom db_table, but should deal with everything else. Thoughts?

lovelydinosaur avatar Jul 31 '13 15:07 lovelydinosaur

Aside: Hmmm. Seems we do the opposite in REST framework. https://github.com/tomchristie/django-rest-framework/blob/master/rest_framework/authtoken/migrations/0001_initial.py

lovelydinosaur avatar Jul 31 '13 15:07 lovelydinosaur

I have to leave for a bit but i'm pretty pragmatic about these things. Custom db_tales is perhaps not something a migration would have to cover although it's sort off nice that it does. Your call?

tijs avatar Jul 31 '13 15:07 tijs

@tomchristie searching through github code i'm getting a sense that the general concensus is a separate compat.py file containing:

try:
    from django.contrib.auth import get_user_model
except ImportError: # django < 1.5
    from django.contrib.auth.models import User
else:
    User = get_user_model()

Which you can then include where needed. In this case for the migrations but also in place of some of the other code in the rest of the compatibility code.

Your version while a bit prettier seems to be in the minority amongst projects on github. Not that this is a democracy but lacking better arguments this seems to be something to go by perhaps?

tijs avatar Jul 31 '13 17:07 tijs