#7584 closed (invalid)
AUTH_PROFILE_MODULE should be able to provide some default profile
Reported by: | Owned by: | ||
---|---|---|---|
Component: | contrib.auth | Version: | dev |
Severity: | Keywords: | ||
Cc: | Triage Stage: | Unreviewed | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Currently if there are users without a profile in the database django raises an DoesNotExist-exception. This exception has to be cought everywhere some application wants to access the profile, which leads to a lot of duplicate code...at least if you use the profile a lot inside your project.
This could easily be fixed by adding some way to auto-create a default profile if necessary and possible. One suggestion might be defining some API that could be called, for example:
- The model (or model-manager) provides some method, that could be called if the profile does not exist
- Otherwise the DoesNotException gets raised like before
I propose changing User.get_profile() so it supports such an API. Patch using model._default_manager.create_default_profile() follows.
Attachments (2)
Change History (10)
by , 16 years ago
Attachment: | django-default-profile.patch added |
---|
follow-up: 2 comment:1 by , 16 years ago
comment:2 by , 16 years ago
Replying to julien:
I don't quite like this approach and I wouldn't do anything in Django core for that issue.
What you want to achieve can already be done using signals:
Signals can only be used to create the profile when saving the user. This is a totally different place to put code. Of course, it might work for many cases, but if some profile gets lost (for example you have some lecagy users) you end up catching the exception everywhere. I think having an easy solution inside the core is a much cleaner solution here, as it really gets all possible scenarios.
follow-up: 4 comment:3 by , 16 years ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
I'm going to wontfix it for the same reasons as in #7592
Your points are valid except that they are not standard. For example, if for some reason the link between a user and its profile was broken, I'd prefer to be notified by an exception (so I can then try to fix the broken link) rather than having the system silently create a fresh new profile.
In your case, you could use a manager:
class ProfileManager(Manager): def get_for_user(self, user): try: return user.get_profile() except: # Create and return a new profile
follow-up: 5 comment:4 by , 16 years ago
Replying to julien:
I'm going to wontfix it for the same reasons as in #7592
So rewriting - or copying - get_profile() everywhere, needing to duplicate code in every application you write (hey, applications should be reusable, right?) or creating your own Profile-application is the way to go here? Sounds not very DRYish to me...
But I'm ok with that if thats the clean (django-)way in your opinion. I'm only trying to change things that might be useful for people new to django. I can work around this, like below.
Replying to julien:
In your case, you could use a manager:
I really could use a manager, true....but this would not work with get_profile(). Using a Manager was my first idea, too. I even started implementing such a Manager, so:
For doing it right (so that get_profile() works) it needs something like this:
class ProfileManager(models.Manager): def get(self, *args, **kwargs): try: return super(ProfileManager, self).get(*args, **kwargs) except: # make sure method-call looks like done from get_profile() if len(args) == 0 and len(kwargs) == 1 and 'user__id__exact' in kwargs: user_id = kwargs['user__id__exact'] p = Profile(user_id = user_id) # add more stuff if necessary p.save() return p else: raise
...not so easy after all. But thats nothing somebody new to django would come up with. Additionaly there may be changes in get_profile() some day (__exact not needed, __id should not be needed, too). So this might break.
Perhaps a different solution would be to support get_for_user() in the manager and only using get(useridexact=self.id) if this method isn't present? This way you could do even more than just creating default profiles.
Meaning get_profile could look like this:
def get_profile(self): """ Returns site-specific profile for this user. Raises SiteProfileNotAvailable if this site does not allow profiles. """ if not hasattr(self, '_profile_cache'): from django.conf import settings if not settings.AUTH_PROFILE_MODULE: raise SiteProfileNotAvailable try: app_label, model_name = settings.AUTH_PROFILE_MODULE.split('.') model = models.get_model(app_label, model_name) if hasattr(model._default_manager, 'get_for_user'): self._profile_cache = model._default_manager.get_for_user(self) else: self._profile_cache = model._default_manager.get(user__id__exact=self.id) except (ImportError, ImproperlyConfigured): raise SiteProfileNotAvailable return self._profile_cache
comment:5 by , 16 years ago
Replying to David Danier <goliath.mailinglist@gmx.de>:
Perhaps a different solution would be to support get_for_user() in the manager and only using get(useridexact=self.id) if this method isn't present? This way you could do even more than just creating default profiles.
btw this would fix #7400, too ;-)
comment:6 by , 16 years ago
David,
I reckon you should raise that question on the dev-list. You would reach a larger audience and probably get more feedback.
Best,
Julien ;)
by , 16 years ago
Attachment: | django-profile-manager.patch added |
---|
New patch implementing different idea of solving this
comment:7 by , 16 years ago
Profile is optional. If you use an application that uses profiles you must also use an application that provides them. There are plenty of profile implementations out there
I don't quite like this approach and I wouldn't do anything in Django core for that issue.
What you want to achieve can already be done using signals: