From 0b494d055e958c91e98cc4bd2823e035bc140860 Mon Sep 17 00:00:00 2001
From: Matthijs Kooijman <matthijs@stdin.nl>
Date: Sun, 10 Feb 2008 15:34:39 +0100
Subject: [PATCH]  * Don't check if a new influence's character belongs to the
 currently logged in player, form.is_valid now handles this.  * Add some
 comments.

---
 influences/views.py | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/influences/views.py b/influences/views.py
index cd8d4a9..85124e4 100644
--- a/influences/views.py
+++ b/influences/views.py
@@ -29,24 +29,27 @@ def add(request, character_id=None):
     # Get the current user's characters
     chars = request.user.character_set.all()
 
+    # If a character_id was specified in the url, or there is only one
+    # character, preselect it.
     if (character_id):
         initial['character'] = character_id
     elif (chars.count() == 1):
         initial['character'] = chars[0].id
 
+
     f = InfluenceForm(request=request, initial=initial)
-    if (f.is_valid()):
-        influence = f.save(commit=False)
-        if (influence.character.player == request.user):
-                influence.save()
-                return HttpResponseRedirect(reverse('influences_influence_detail', args=[influence.id]))
-        else:
-                # TODO: Make this a bit more pretty. Perhaps throw an exception here and add some middleware to catch it?
-                return HttpResponseForbidden("Forbidden -- Trying to submit influence for somebody else's character")
-     
-    # Only allow characters of the current user
+
+    # Only allow characters of the current user. Putting this here also
+    # ensures that a form will not validate when any other choice was
+    # selected (perhaps through URL crafting).
     f.fields['character']._set_queryset(chars)
 
+    if (f.is_valid()):
+        # The form was submitted, let's save it.
+        influence = f.save()
+        # Redirect to the just saved influence
+        return HttpResponseRedirect(reverse('influences_influence_detail', args=[influence.id]))
+
     return render_to_response('influences/add.html', {'form' : f}, RequestContext(request))
 
 @login_required
@@ -67,26 +70,37 @@ def index(request):
     influences = Influence.objects.filter(character__player=request.user)
     return render_to_response('influences/index.html', {'characters' : characters, 'influences' : influences}, RequestContext(request))
 
+#
+# The views below are very similar to django's generic views (in fact,
+# they used to be generic views before). However, since they all depend
+# on the currently logged in user (for limiting the show list or
+# performing access control), we won't actually use the generic views
+# here.
+
 @login_required
 def character_list(request):
+    # Only show this player's characters
     os = request.user.character_set.all()
     return render_to_response('influences/character_list.html', {'object_list' : os}, RequestContext(request))
 
 @login_required
 def character_detail(request, object_id):
     o = Character.objects.get(pk=object_id)
+    # Don't show other player's characters
     if (o.player != request.user):
         return HttpResponseForbidden("Forbidden -- Trying to view somebody else's character")
     return render_to_response('influences/character_detail.html', {'object' : o}, RequestContext(request))
 
 @login_required
 def influence_list(request):
+    # Only show this player's influences
     os = Influence.objects.filter(character__player=request.user)
     return render_to_response('influences/influence_list.html', {'object_list' : os}, RequestContext(request))
 
 @login_required
 def influence_detail(request, object_id):
     o = Influence.objects.get(pk=object_id)
+    # Don't show other player's influences
     if (o.character.player != request.user):
         return HttpResponseForbidden("Forbidden -- Trying to view influences of somebody else's character")
     return render_to_response('influences/influence_detail.html', {'object' : o}, RequestContext(request))
-- 
2.30.2