From 3b2265e869fc180bf74af6573a875fd6a916cced Mon Sep 17 00:00:00 2001
From: Andreas Kloeckner <inform@tiker.net>
Date: Mon, 29 Dec 2014 01:02:41 +0100
Subject: [PATCH] Never store current flow session in cookie
---
TODO | 2 -
course/flow.py | 85 ++++++++-----------
course/models.py | 9 +-
.../course/flow-completion-grade.html | 5 +-
course/templates/course/flow-completion.html | 10 +--
.../course/flow-confirm-completion.html | 2 +-
course/templates/course/flow-page.html | 8 +-
courseflow/urls.py | 8 +-
8 files changed, 51 insertions(+), 78 deletions(-)
diff --git a/TODO b/TODO
index 974f0afc..e4ef7e45 100644
--- a/TODO
+++ b/TODO
@@ -10,8 +10,6 @@
- MOSS submission
-- Don't store current flow session in cookie, ever.
-
- Redo permissions system
- Custom matcher classes
diff --git a/course/flow.py b/course/flow.py
index d0dc369c..42d2828b 100644
--- a/course/flow.py
+++ b/course/flow.py
@@ -61,15 +61,6 @@ from course.utils import (
from course.views import get_now_or_fake_time
-def get_flow_session_id_map(request):
- result = request.session.setdefault("flow_session_id_map", {})
-
- # Tell Django: This session has been modified, persist it.
- request.session["flow_session_id_map"] = result
-
- return result
-
-
# {{{ grade page visit
def grade_page_visit(visit, visit_grade_model=FlowPageVisitGrade,
@@ -739,11 +730,9 @@ def start_flow(pctx, flow_identifier):
or resume_session.in_progress):
raise PermissionDenied("not allowed to resume session")
- get_flow_session_id_map(request)[flow_identifier] = resume_session_id
-
return redirect("course.flow.view_flow_page",
pctx.course.identifier,
- flow_identifier,
+ resume_session_id,
0)
elif ("start_no_credit" in request.POST
@@ -789,17 +778,13 @@ def start_flow(pctx, flow_identifier):
get_flow_grading_opportunity(
pctx.course, flow_identifier, fctx.flow_desc)
- get_flow_session_id_map(request)[flow_identifier] = session.id
-
page_count = set_up_flow_session_page_data(fctx.repo, session,
pctx.course.identifier, fctx.flow_desc, pctx.course_commit_sha)
session.page_count = page_count
session.save()
return redirect("course.flow.view_flow_page",
- pctx.course.identifier,
- flow_identifier,
- 0)
+ pctx.course.identifier, session.id, 0)
else:
raise SuspiciousOperation("unrecognized POST action")
@@ -860,21 +845,24 @@ def start_flow(pctx, flow_identifier):
# {{{ view: flow page
-def find_current_flow_session(request, course, flow_identifier):
- flow_session = None
- flow_session_id = get_flow_session_id_map(request).get(flow_identifier)
-
- if flow_session_id is not None:
- try:
- flow_session = FlowSession.objects.get(id=flow_session_id)
- except ObjectDoesNotExist:
- return None
+def get_and_check_flow_session(pctx, flow_session_id):
+ try:
+ flow_session = FlowSession.objects.get(id=flow_session_id)
+ except ObjectDoesNotExist:
+ raise http.Http404()
- if flow_session.course.pk != course.pk:
- return None
+ if pctx.role in [
+ participation_role.instructor,
+ participation_role.teaching_assistant]:
+ pass
+ elif pctx.role == participation_role.student:
+ if pctx.participation != flow_session.participation:
+ raise PermissionDenied("may not view other people's sessions")
+ else:
+ raise PermissionDenied()
- if flow_session.flow_id != flow_identifier:
- return None
+ if flow_session.course.pk != pctx.course.pk:
+ raise SuspiciousOperation()
return flow_session
@@ -935,11 +923,13 @@ def create_flow_page_visit(request, flow_session, page_data):
@course_view
-def view_flow_page(pctx, flow_identifier, ordinal):
+def view_flow_page(pctx, flow_session_id, ordinal):
request = pctx.request
- flow_session = find_current_flow_session(
- request, pctx.course, flow_identifier)
+ flow_session_id = int(flow_session_id)
+ flow_session = get_and_check_flow_session(
+ pctx, flow_session_id)
+ flow_identifier = flow_session.flow_id
if flow_session is None:
messages.add_message(request, messages.WARNING,
@@ -996,7 +986,7 @@ def view_flow_page(pctx, flow_identifier, ordinal):
if request.method == "POST":
if "finish" in request.POST:
return redirect("course.flow.finish_flow_session_view",
- pctx.course.identifier, flow_identifier)
+ pctx.course.identifier, flow_session_id)
else:
# reject answer update if flow is not in-progress
if not flow_session.in_progress:
@@ -1071,12 +1061,12 @@ def view_flow_page(pctx, flow_identifier, ordinal):
and not will_receive_feedback(permissions)):
return redirect("course.flow.view_flow_page",
pctx.course.identifier,
- flow_identifier,
+ flow_session_id,
fpctx.ordinal + 1)
elif (pressed_button == "save_and_finish"
and not will_receive_feedback(permissions)):
return redirect("course.flow.finish_flow_session_view",
- pctx.course.identifier, flow_identifier)
+ pctx.course.identifier, flow_session_id)
else:
form = fpctx.page.make_form(
page_context, page_data.data,
@@ -1286,22 +1276,15 @@ def update_expiration_mode(pctx, flow_session_id):
@transaction.atomic
@course_view
-def finish_flow_session_view(pctx, flow_identifier):
+def finish_flow_session_view(pctx, flow_session_id):
now_datetime = get_now_or_fake_time(pctx.request)
request = pctx.request
- flow_session = find_current_flow_session(
- request, pctx.course, flow_identifier)
-
- if flow_session is None:
- messages.add_message(request, messages.WARNING,
- "No session record found for this flow. "
- "Redirected to flow start page.")
-
- return redirect("course.flow.start_flow",
- pctx.course.identifier,
- flow_identifier)
+ flow_session_id = int(flow_session_id)
+ flow_session = get_and_check_flow_session(
+ pctx, flow_session_id)
+ flow_identifier = flow_session.flow_id
fctx = FlowContext(pctx.repo, pctx.course, flow_identifier,
participation=pctx.participation,
@@ -1343,9 +1326,6 @@ def finish_flow_session_view(pctx, flow_identifier):
if not flow_session.in_progress:
raise PermissionDenied("Can't end a session that's already ended")
- # Actually end the flow session
- get_flow_session_id_map(request)[flow_identifier] = None
-
grade_info = finish_flow_session(
fctx, flow_session, current_access_rule,
now_datetime=now_datetime)
@@ -1360,6 +1340,7 @@ def finish_flow_session_view(pctx, flow_identifier):
return render_finish_response(
"course/flow-completion.html",
last_page_nr=None,
+ flow_session=flow_session,
completion_text=completion_text)
if not is_graded_flow:
@@ -1368,6 +1349,7 @@ def finish_flow_session_view(pctx, flow_identifier):
return render_finish_response(
"course/flow-completion.html",
last_page_nr=flow_session.page_count-1,
+ flow_session=flow_session,
completion_text=completion_text)
elif not flow_session.in_progress:
@@ -1384,6 +1366,7 @@ def finish_flow_session_view(pctx, flow_identifier):
return render_finish_response(
"course/flow-confirm-completion.html",
last_page_nr=flow_session.page_count-1,
+ flow_session=flow_session,
answered_count=answered_count,
unanswered_count=unanswered_count,
total_count=answered_count+unanswered_count)
diff --git a/course/models.py b/course/models.py
index a906b16f..7b2ddf5d 100644
--- a/course/models.py
+++ b/course/models.py
@@ -98,7 +98,8 @@ class Course(models.Model):
"<tt>git://github.com/inducer/courseflow-sample</tt> "
"to get some sample content.")
ssh_private_key = models.TextField(blank=True,
- help_text="An SSH private key to use for Git authentication")
+ help_text="An SSH private key to use for Git authentication. "
+ "Not needed for the sample URL above.")
course_file = models.CharField(max_length=200,
default="course.yml",
@@ -154,6 +155,8 @@ class Course(models.Model):
# }}}
+# {{{ event
+
class Event(models.Model):
"""An event is an identifier that can be used to specify dates in
course content.
@@ -183,6 +186,8 @@ class Event(models.Model):
else:
return self.kind
+# }}}
+
# {{{ participation
@@ -482,8 +487,6 @@ def get_feedback_for_grade(grade):
# }}}
-# }}}
-
# {{{ flow access
diff --git a/course/templates/course/flow-completion-grade.html b/course/templates/course/flow-completion-grade.html
index 8c86e146..ac7fe512 100644
--- a/course/templates/course/flow-completion-grade.html
+++ b/course/templates/course/flow-completion-grade.html
@@ -63,10 +63,7 @@
<div class="well flow-well">
<a class="btn btn-default"
href="{% url "course.flow.start_flow" course.identifier flow_identifier %}"
- role="button">« Start over / Review</a>
- <a class="btn btn-default"
- href="{% url "course.views.course_page" course.identifier %}"
- role="button">To course page »</a>
+ role="button">« Back to start page</a>
</div>
{% endblock %}
diff --git a/course/templates/course/flow-completion.html b/course/templates/course/flow-completion.html
index 1e572905..a9c663ef 100644
--- a/course/templates/course/flow-completion.html
+++ b/course/templates/course/flow-completion.html
@@ -8,17 +8,9 @@
{{ completion_text|safe }}
<div class="well flow-well">
- {% if last_page_nr %}
- <a class="btn btn-default"
- href="{% url "course.flow.view_flow_page" course.identifier flow_identifier last_page_nr %}"
- role="button">« Go back</a>
- {% endif %}
<a class="btn btn-default"
href="{% url "course.flow.start_flow" course.identifier flow_identifier %}"
- role="button">« Start over</a>
- <a class="btn btn-default"
- href="{% url "course.views.course_page" course.identifier %}"
- role="button">To course page »</a>
+ role="button">« Back to start page</a>
</div>
{% endblock %}
diff --git a/course/templates/course/flow-confirm-completion.html b/course/templates/course/flow-confirm-completion.html
index 82ebbb89..3fa3d7a2 100644
--- a/course/templates/course/flow-confirm-completion.html
+++ b/course/templates/course/flow-confirm-completion.html
@@ -36,7 +36,7 @@
{% csrf_token %}
<a class="btn btn-default"
- href="{% url "course.flow.view_flow_page" course.identifier flow_identifier last_page_nr %}"
+ href="{% url "course.flow.view_flow_page" course.identifier flow_session.id last_page_nr %}"
role="button">« Go back</a>
<button type="submit" name="submit" class="btn btn-primary">Confirm: Submit answers and end session</button>
diff --git a/course/templates/course/flow-page.html b/course/templates/course/flow-page.html
index bf2e094d..b01469a9 100644
--- a/course/templates/course/flow-page.html
+++ b/course/templates/course/flow-page.html
@@ -10,7 +10,7 @@
{% block content %}
<form
- action="{% url "course.flow.view_flow_page" course.identifier flow_identifier page_data.ordinal %}"
+ action="{% url "course.flow.view_flow_page" course.identifier flow_session.id page_data.ordinal %}"
class="navigation-form"
method="POST">
@@ -20,7 +20,7 @@
<div class="flow-nav">
{% if page_data.ordinal > 0 %}
<a class="btn btn-default cf-nav-button"
- href="{% url "course.flow.view_flow_page" course.identifier flow_identifier page_data.previous_ordinal %}"
+ href="{% url "course.flow.view_flow_page" course.identifier flow_session.id page_data.previous_ordinal %}"
role="button">« Previous</a>
{% else %}
<a class="btn btn-default cf-nav-button disabled"
@@ -29,7 +29,7 @@
{% endif %}
{% if page_data.next_ordinal < flow_session.page_count %}
<a class="btn btn-default cf-nav-button"
- href="{% url "course.flow.view_flow_page" course.identifier flow_identifier page_data.next_ordinal %}"
+ href="{% url "course.flow.view_flow_page" course.identifier flow_session.id page_data.next_ordinal %}"
role="button">Next »</a>
{% else %}
<a class="btn btn-default cf-nav-button disabled"
@@ -63,7 +63,7 @@
{% if page_nr == page_data.ordinal %}
<span class="cf-flow-page-number cf-current btn btn-default btn-xs disabled">{{ shown_page_nr }}</span>
{% else %}
- <a href="{% url "course.flow.view_flow_page" course.identifier flow_identifier page_nr %}">
+ <a href="{% url "course.flow.view_flow_page" course.identifier flow_session.id page_nr %}">
<span class="cf-flow-page-number btn btn-default btn-xs
{% if page_nr < page_data.ordinal %}
cf-prior
diff --git a/courseflow/urls.py b/courseflow/urls.py
index 110a0c66..253b66ed 100644
--- a/courseflow/urls.py
+++ b/courseflow/urls.py
@@ -216,8 +216,8 @@ urlpatterns = patterns('',
"course.flow.start_flow",),
url(r"^course"
"/(?P<course_identifier>[-a-zA-Z0-9]+)"
- "/flow"
- "/(?P<flow_identifier>[-_a-zA-Z0-9]+)"
+ "/flow-session/"
+ "/(?P<flow_session_id>[0-9]+)"
"/(?P<ordinal>[0-9]+)"
"/$",
"course.flow.view_flow_page",),
@@ -230,8 +230,8 @@ urlpatterns = patterns('',
"course.flow.update_expiration_mode",),
url(r"^course"
"/(?P<course_identifier>[-a-zA-Z0-9]+)"
- "/flow"
- "/(?P<flow_identifier>[-_a-zA-Z0-9]+)"
+ "/flow-session"
+ "/(?P<flow_session_id>[0-9]+)"
"/finish"
"/$",
"course.flow.finish_flow_session_view",),
--
GitLab