views:

67

answers:

2

If I have a view that handles the management of friends, meaning there is a view to handle adding, removing, blocking, unblocking, and accepting/denying invitations to become friends. The problem I have run into is when I try to provide meaningful errors to users who end up at a url they shouldn't be at.

For example if User1 and User2 are already friends and User1 goes to the url for adding User2 as a friend instead of the form being presented as if they weren't friends and the form failing on unique_together = (('user_from', 'user_to'),) it displays a warning message and redirects them to an appropriate page before the form is displayed.

Like this

def add_friend(request, username):
    try:
        user = User.objects.get(username=username)
    except User.DoesNotExist:
        messages.error(request, 'A user with the username %s does not exist. \
            Try searching for the user below.' % username)
        return HttpResponseRedirect(reverse('friends_find_friend'))

    if Friend.objects.are_friends(request.user, user):
        messages.error(request, 'You are already friends with %s' % user)
        return HttpResponseRedirect(reverse('profiles_profile_detail', args=[user]))

This also includes a check and meaningfull error message (instead of 404) if there is no such user.

That is easy to handle but with other checks it grows to

def add_friend(request, username):
    try:
        user = User.objects.get(username=username)
    except User.DoesNotExist:
        messages.error(request, 'A user with the username %s does not exist. \
            Try searching for the user below.' % username)
        return HttpResponseRedirect(reverse('friends_find_friend'))

    if user == request.user:
        messages.error(request, 'You are already friends with yourself')
        return HttpResponseRedirect(reverse('friends_find_friend'))

    if Enemy.objects.is_blocked(request.user, user):
        messages.error(request, '%s has blocked you from adding them as a friend' % user)
        return HttpResponseRedirect(reverse('friends_find_friend'))

    if Enemy.objects.has_blocked(request.user, user):
        messages.error(request, 'You have blocked %s so you cannot add them as a friend' % user)
        return HttpResponseRedirect(reverse('profiles_profile_detail', args=[user]))

    if Friend.objects.are_friends(request.user, user):
        messages.error(request, 'You are already friends with %s' % user)
        return HttpResponseRedirect(reverse('profiles_profile_detail', args=[user]))

    if FriendRequest.objects.invitation_sent(request.user, user):
        messages.error(request, 'You already sent %s a request. You need to \
            wait for them to reply to it.' % user)
        return HttpResponseRedirect(reverse('friends_pending'))

    if FriendRequest.objects.invitation_received(request.user, user):
        messages.error(request, '%s already sent you a request and is waiting \
            for you to respond to them.' % user)
        return HttpResponseRedirect(reverse('friends_pending'))

All of this is duplicated again in

  • remove_friend
  • block_user
  • unblock_user
  • pending_invitations

And further duplicated as form validation errors in case the view is bypassed in the shell and the form is used independently.

What I am asking is if there is a more pythonic way to accomplish this without the excessive copying and pasting?

Edit:

I've been trying to solve this and was wondering if something like this would be a good way.

tests = ((Enemy.objects.is_blocked, 'This user has blocked you', reverse('friends_find_friend')),
         (Enemy.objects.has_blocked, 'You have blocked this user', reverse('profiles_profile_detail', args=[user])),)

for test in tests:
    if test[0](request.user, user):
        messages.error(request, test[1])
        return HttpResponseRedirect(test[2])

The tests would be defined in another file similar to url patterns and a decorator would wrap the view function to run over all the tests, redirect if anything fails and finally pass it off to the view function if everything is good. Would this be an effective method way to manage without clogging up the view file with hundreds of lines of boilerplate code?

Edit2:

I'm also interested in seeing how others do similar things. I doubt I'm the first person who wants to display a message to the user instead of just throwing a 404 page.

A: 

I imagine that code is not entirely duplicated inside remove_friend and block_user etc (since the messages would be different) - you can probably refactor some of your function into separate helper functions which take two users and return some kind of status indicating success or failure.

It's difficult to be more helpful without a sample of say your pending_invitations view (which seems likely to be the most different to add_friend).

Dominic Rodger
The `pending_invitations` just checks which submit button was pressed (confirm or deny) and acts pretty much like another two views called `accept` or `deny` with the same checks to ensure that the user exists, there is a pending request, you aren't trying to accept the request you sent, etc.
Rigsby
+1  A: 

Python decorators are perfect for this.

Define all the validations you are doing within the decorator and decorate all your add_friend, remove_friend etc, with that.

Update:

For your case it is going to look something like this:

def do_friending_validation(fun):
 def validate_function(*args,**kwargs):
  try:
   user = User.objects.get(username=username)
  except User.DoesNotExist:
   messages.error(request, 'A user with the username %s does not exist. \
       Try searching for the user below.' % username)
   return HttpResponseRedirect(reverse('friends_find_friend'))

  if user == request.user:
  messages.error(request, 'You are already friends with yourself')
  return HttpResponseRedirect(reverse('friends_find_friend'))

  if Enemy.objects.is_blocked(request.user, user):
  messages.error(request, '%s has blocked you from adding them as a friend' % user)
  return HttpResponseRedirect(reverse('friends_find_friend'))

  if Enemy.objects.has_blocked(request.user, user):
  messages.error(request, 'You have blocked %s so you cannot add them as a friend' % user)
  return HttpResponseRedirect(reverse('profiles_profile_detail', args=[user]))

  if Friend.objects.are_friends(request.user, user):
  messages.error(request, 'You are already friends with %s' % user)
  return HttpResponseRedirect(reverse('profiles_profile_detail', args=[user]))

  if FriendRequest.objects.invitation_sent(request.user, user):
  messages.error(request, 'You already sent %s a request. You need to \
      wait for them to reply to it.' % user)
  return HttpResponseRedirect(reverse('friends_pending'))

  if FriendRequest.objects.invitation_received(request.user, user):
  messages.error(request, '%s already sent you a request and is waiting \
      for you to respond to them.' % user)
  return HttpResponseRedirect(reverse('friends_pending'))
  fun(*args,**kwargs)

 return validate_function

@do_friending_validation
def add_friend(request, username):

 #Do your stuff here

@do_friending_validation 
def remove_friend(request, username):

 #Do your stuff here
Lakshman Prasad
I thought of using decorators but the only verification that remains exactly the same is the first user one. The messages in the others will very slightly (i.e. you are already friends in `add_friends` and you aren't yet friends in `remove_friends`.
Rigsby
then just set error flags in the decorator and select messages based on the flags decorator raised in the view.
kibitzer