Code

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#7584 closed (invalid)

AUTH_PROFILE_MODULE should be able to provide some default profile

Reported by: David Danier <goliath.mailinglist@…> Owned by:
Component: contrib.auth Version: master
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

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)

django-default-profile.patch (993 bytes) - added by David Danier <goliath.mailinglist@…> 6 years ago.
django-profile-manager.patch (911 bytes) - added by David Danier <goliath.mailinglist@…> 6 years ago.
New patch implementing different idea of solving this

Download all attachments as: .zip

Change History (10)

Changed 6 years ago by David Danier <goliath.mailinglist@…>

comment:1 follow-up: Changed 6 years ago by julien

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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:

def create_profile_for_user(sender, instance, signal, created, *args, **kwargs):
    from profiles.models import UserProfile
    if created:
        profile = UserProfile(user = instance)
        profile.save()

dispatcher.connect(create_profile_for_user, signal=signals.post_save, sender=User)

comment:2 in reply to: ↑ 1 Changed 6 years ago by David Danier <goliath.mailinglist@…>

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.

comment:3 follow-up: Changed 6 years ago by julien

  • Resolution set to invalid
  • Status changed from new to 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

comment:4 in reply to: ↑ 3 ; follow-up: Changed 6 years ago by David Danier <goliath.mailinglist@…>

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 in reply to: ↑ 4 Changed 6 years ago by David Danier <goliath.mailinglist@…>

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 Changed 6 years ago by julien

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 ;)

Changed 6 years ago by David Danier <goliath.mailinglist@…>

New patch implementing different idea of solving this

comment:7 Changed 6 years ago by hvendelbo

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

comment:8 Changed 6 years ago by guettli

See patch for the documentation: #9987

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.