Merge lp://staging/~ricardokirkner/django-openid-auth/custom-user-groups into lp://staging/django-openid-auth

Proposed by Ricardo Kirkner
Status: Merged
Approved by: Ricardo Kirkner
Approved revision: 130
Merged at revision: 125
Proposed branch: lp://staging/~ricardokirkner/django-openid-auth/custom-user-groups
Merge into: lp://staging/django-openid-auth
Diff against target: 232 lines (+116/-12)
5 files modified
django_openid_auth/auth.py (+50/-6)
django_openid_auth/tests/models.py (+9/-0)
django_openid_auth/tests/test_auth.py (+49/-3)
setup.py (+2/-2)
tox.ini (+6/-1)
To merge this branch: bzr merge lp://staging/~ricardokirkner/django-openid-auth/custom-user-groups
Reviewer Review Type Date Requested Status
Fabián Ezequiel Gallina (community) Approve
Daniel Manrique (community) Approve
Review via email: mp+315163@code.staging.launchpad.net

Commit message

support custom user group model

also properly support django 1.6 up to 1.10

To post a comment you must log in.
Revision history for this message
Daniel Manrique (roadmr) wrote :

LGTM code-wise, I'm not very knowledgeable on the subtleties of django-openid-auth but this seems to make sense. +1 but maybe you want another review. OTOH this is simple enough that if your tests show it works, we could go ahead.

review: Approve
Revision history for this message
Daniel Manrique (roadmr) wrote :

The explanation was useful to me:

14:42:52 pindonga django.contrib.auth includes User and Group
14:43:02 pindonga and User has a m2m field groups
14:43:22 pindonga django-openid-auth relied on the contrib models so far
14:43:46 pindonga first we allowed for a custom user model, but that would still require that model to include a groups m2m to Group
14:43:54 pindonga sentry's custom user model doesn't have that
14:43:59 pindonga it doesn't use the groups at all
14:44:19 pindonga but to keep d-o-a working like now we do use the group model
14:44:41 pindonga so I though of allowing to customize the <custom user>-group relation by allowing a custom <user, group> mapping table
14:57:39 roadmr pindonga: hm... so in our Sentry's settings you define AUTH_USER_GROUP_MODEL to keep d-o-a happy?
14:57:48 pindonga exactly
14:58:05 pindonga with a custom model provided by sentry_auth_ubuntu_sso (our sso plugin for sentry)

This makes sense and the default behavior to use User.groups.through model is consistent with the explanation.

LGTM :) Thanks!

review: Approve
Revision history for this message
Fabián Ezequiel Gallina (fgallina) :
Revision history for this message
Ricardo Kirkner (ricardokirkner) :
130. By Ricardo Kirkner

fixes per review

Revision history for this message
Fabián Ezequiel Gallina (fgallina) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches