2
\$\begingroup\$

I'm in the process of writing a private messaging system and I have 2 URLs which go to the same view (is that bad?). The logic behind this madness relates to the way user relationships are structured in our system...

     T
    / \___
   C1      \
   |        C2
   P1      /  \  
          P2   P3

T = Trainer, C = Coach, P = Participant.

We define this structure in a pretty simple way:

class UserProfile(models.Model):
    user = models.OneToOneField(User)
    higher_up = models.ForeignKey('self', blank=True, null=True)
    # ...
    type = models.CharField(max_length=1, choices=TYPES, default='P')

And bam, We've got a DAG. Here's my 2 urls in urls.py:

# talk to lower_down
url(r'^pm/(?P<userprofile_pk>\d+)/$', 'private_message'),

# talk to higher_up
url(r'^pm/$', 'private_message'),

and the private_message view...

def private_message(request, userprofile_pk=''):
    me = request.user.userprofile
    src_pk = me.pk

    if userprofile_pk == '': # /pm/
        you = me.higher_up
        dest_pk = you.pk
    else: # userprofile_pk was not blank /pm/<num>
        you = UserProfile.objects.get(pk=userprofile_pk) 
        dest_pk = you.pk

    # Are they allowed to talk?
    if me.is_assoc_with(you):
        return HttpResponseRedirect('/')

    feed_vars = {
        'dest_pk': dest_pk,
        'src_pk' : src_pk,
    }

    return render(request, 'talk/private_message.html', feed_vars)

The me.is_assoc_with(you) bit just checks where me and you are in the userprofile structure. In the above ASCII art diagram, P1 should not be allowed to talk with P2. T can talk to C1, C1 and C2 cannot talk. T and P3 can talk, etc.

Is this an ok pattern to have 2 URLs go to 1 view that handles logic based on the URL?

I do feel like I am reinventing the wheel here. I think there should be a more flexible approach with settings for who can talk to whom factored out of the business logic itself.

\$\endgroup\$
1
\$\begingroup\$

Why not separate the view functions?

# talk to lower_down
url(r'^pm/(?P<userprofile_pk>\d+)/$', 'private_message_down'),

# talk to higher_up
url(r'^pm/$', 'private_message_up'),

The view functions:

def private_message(src, dest):
    # Are they allowed to talk?
    if src.is_assoc_with(dest):
        return HttpResponseRedirect('/')

    feed_vars = {
        'dest_pk': dest.pk,
        'src_pk' : src.pk,
    }

    return render(request, 'talk/private_message.html', feed_vars)

def private_message_down(request, userprofile_pk):
    src = request.user.userprofile

    # TODO: profile actually exists?
    dest = UserProfile.objects.get(pk=userprofile_pk) 

    return private_message(src, dest)

def private_message_up(request):
    src = request.user.userprofile
    dest = me.higher_up

    return private_message(src, dest)

I added a TODO for failure handling, I think you should wrap the .get lookup in a try-except UserProfile.DoesNotExist, for example:

from django.http import Http404


def private_message_down(request, userprofile_pk):
    src = request.user.userprofile

    try:
        dest = UserProfile.objects.get(pk=userprofile_pk) 
    catch UserProfile.DoesNotExist:
        raise Http404

    return private_message(src, dest)

Also, you probably want to decorate private_message_* with @login_required (from django.contrib.auth.decorators import login_required).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.