Load SCORM data through fetch instead of embedding it in the page

fixes #266

The run_attempt view doesn't embed the SCORM data model in the HTML any
more. Instead, the client makes a fetch request to a new `/run_attempt/<pk>/scorm_cmi`
view, which returns the current SCORM data.

This ensures that even if the page has reloaded from the browser's
cache, it will only start with the latest data.
If the data can't be loaded for some reason, such as no network
connection, the student is shown an error message and a button to reload
the page.

This commit also makes the run attempt and scorm cmi views throw a
PermissionDenied error if the attempt would be opened in review mode,
but review is not allowed yet, and the user is not an instructor.
Previously, a student could open an attempt in review mode by reloading
the page, even when it's not allowed yet.

An AttemptLaunch record is created each time the SCORM data model is
requested. This should give more detail about when a student reloads an
attempt by browser navigation instead of by clicking links.

numbas_lti/static/api.js

@@ -199,7 +199,8 @@ SCORM_API.prototype = {
         for(var id in stored_data.sent) {
             this.sent[id] = {
                 elements: stored_data.sent[id].map(function(e) {
-                    return new SCORMData(e.key,e.value,DateTime.fromSeconds(e.time));
+                    var time = e.time_iso !== undefined ? DateTime.fromISO(e.time_iso) : DateTime.fromSeconds(e.time);
+                    return new SCORMData(e.key, e.value, time);
                 }),
                 time: DateTime.utc()
             }

numbas_lti/static/attempt_timeline.js

@@ -307,9 +307,9 @@ Timeline.prototype = {
     add_launch: function(launch) {
         var msg;
         if(launch.user!=null) {
-            msg = interpolate(_('Launched in %s mode by %s.'),[launch.mode,launch.user]);
+            msg = interpolate(_('Launched in %s mode by %s.'), [launch.mode,launch.user]);
         } else {
-            msg = interpolate(_('Launched in %s mode.'),launch.mode);
+            msg = interpolate(_('Launched in %s mode.'), [launch.mode]);
         }
         this.add_timeline_item(new TimelineItem(
             msg,

numbas_lti/static/run_attempt.css

@@ -40,7 +40,8 @@ body.terminated #status-display .symbol, #status-display .symbol {
 #status-display.confirmation .text-confirmation,
 #status-display.unavailable-at .text-unavailable-at,
 #status-display.terminated:not(.ok):not(.failed-final-ajax) .text-terminated,
-#status-display.failed-final-ajax .text-failed-final-ajax
+#status-display.failed-final-ajax .text-failed-final-ajax,
+#status-display.loading-error .text-loading-error
 {
     display: block;
 }

numbas_lti/static/run_attempt.js

@@ -0,0 +1,55 @@
+function initialise_api(scorm_cmi) {
+    var scorm_api_data = js_vars.scorm_api_data;
+    scorm_api_data.scorm_cmi = scorm_cmi;
+    try {
+        var sc = new SCORM_API(scorm_api_data);
+        window.API_1484_11 = sc.API_1484_11;
+    } catch(e) {
+        console.error(e);
+        alert(gettext("A connection to the server could not be created. Please report this."));
+        redirect_to_attempts();
+    }
+    if(document.readyState == 'complete') {
+        load_iframe();
+    } else {
+        window.addEventListener('load',function() {
+            load_iframe();
+        });
+    }
+}
+
+function redirect_to_attempts() {
+    window.location.href = js_vars.scorm_api_data.show_attempts_url;
+}
+
+function load_iframe() {
+    var iframe = document.createElement('iframe');
+    iframe.setAttribute('id','scorm-player');
+    iframe.setAttribute('width','100%');
+    iframe.setAttribute('height','100%');
+    iframe.setAttribute('src',js_vars.exam_url);
+    var iframe_container = document.getElementById('scorm-player-container');
+    iframe_container.innerHTML = '';
+    iframe_container.appendChild(iframe);
+}
+
+function show_loading_error(e) {
+    document.body.classList.add('terminated');
+    document.getElementById('scorm-player-container').style.display = 'none';
+    var status_display = document.getElementById('status-display');
+    status_display.className = 'loading-error';
+    console.error(e);
+}
+
+var js_vars = JSON.parse(document.getElementById('js_vars').textContent);
+
+fetch(js_vars.cmi_url, {headers: {'Accept': 'application/json'}}).then(function(response) {
+    if(response.status == 403) {
+        redirect_to_attempts();
+        return;
+    }
+    if(response.status != 200) {
+        throw new Error(_("There was an error fetching the attempt data."));
+    }
+    response.json().then(function(data) { initialise_api(data); });
+}).catch(show_loading_error);

numbas_lti/templates/numbas_lti/review_not_allowed.html

@@ -0,0 +1 @@
+{% load i18n %}{% blocktranslate with allow_review_from=allow_review_from %}Review will be available from {{allow_review_from}}{% endblocktranslate %}

numbas_lti/templates/numbas_lti/run_attempt.html

@@ -56,6 +56,12 @@
             <button type="button" id="try-final-ajax-again">{% trans "Try again" %}</button>
         </div>
     </span>
+    <span class="status-message text-loading-error">
+        <div class="text">
+            <p>{% trans "There was an error loading the data for this attempt. Please reload the page." %}</p>
+            <button type="button" onclick="location.reload()">{% trans "Reload the page" %}</button>
+        </div>
+    </span>
 </div>
 <div id="deadline-change-display" tabindex="1">
     <p>{% blocktrans %}The availability dates for this attempt have changed. The attempt is available until <span class="available-until"></span>.{% endblocktrans %}</p>
@@ -65,7 +71,7 @@
     <h1 style="text-align: center">Loading "{{attempt.resource.title}}"</h1>
 </div>
 
-{% if scorm_cmi %}
+{% if load_cmi %}
 <script>
 /*! modernizr 3.3.1 (Custom Build) | MIT *
  * https://modernizr.com/download/?-websockets-setclasses !*/
@@ -84,27 +90,7 @@ <h1 style="text-align: center">Loading "{{attempt.resource.title}}"</h1>
 {% include "numbas_lti/scripts/luxon.html" %}
 <script type="text/javascript" src="{% static 'api.js' %}"></script>
 {{js_vars|json_script:"js_vars"}}
-<script type="text/javascript">
-    var js_vars = JSON.parse(document.getElementById('js_vars').textContent);
-    try {
-        var sc = new SCORM_API(js_vars);
-        window.API_1484_11 = sc.API_1484_11;
-    } catch(e) {
-        console.error(e);
-        alert("{% trans "A connection to the server could not be created. Please report this." %}");
-        window.location.href = '{% url 'show_attempts' %}';
-    }
-    window.addEventListener('load',function() {
-        var iframe = document.createElement('iframe');
-        iframe.setAttribute('id','scorm-player');
-        iframe.setAttribute('width','100%');
-        iframe.setAttribute('height','100%');
-        iframe.setAttribute('src',js_vars.exam_url);
-        var iframe_container = document.getElementById('scorm-player-container');
-        iframe_container.innerHTML = '';
-        iframe_container.appendChild(iframe);
-    });
-</script>
+<script type="text/javascript" src="{% static 'run_attempt.js' %}"></script>
 {% endif %}
 
 <script type="text/javascript">

numbas_lti/templates/numbas_lti/show_attempts.html

@@ -75,7 +75,7 @@ <h1>
                             </a>
                             {% else %}
                                 {% if attempt.resource.allow_review_from %}
-                                    {% blocktranslate with time=attempt.resource.allow_review_from %}Review will be available from {{time}}{% endblocktranslate %}
+                                    {% include "numbas_lti/review_not_allowed.html" with allow_review_from=attempt.resource.allow_review_from %}
                                 {% endif %}
                             {% endif %}
                         {% else %}

numbas_lti/urls.py

@@ -60,6 +60,7 @@
     path('show_attempts', views.attempt.ShowAttemptsView.as_view(), name='show_attempts'),
     path('new_attempt', views.attempt.new_attempt, name='new_attempt'),
     path('run_attempt/<int:pk>', views.attempt.RunAttemptView.as_view(), name='run_attempt'),
+    path('run_attempt/<int:pk>/scorm_cmi', views.attempt.AttemptScormCMIView.as_view(), name='run_attempt_scorm_cmi'),
 
     path('no-websockets', views.entry.no_websockets, name='no_websockets'),
     path('not-authorized', views.entry.not_authorized, name='not_authorized'),

numbas_lti/views/attempt.py

@@ -19,7 +19,7 @@
 from numbas_lti.forms import RemarkPartScoreForm
 from numbas_lti.models import Resource, AccessToken, Exam, Attempt, ScormElement, RemarkPart, AttemptLaunch, resolve_diffed_scormelements, RemarkedScormElement
 from numbas_lti.save_scorm_data import save_scorm_data
-from numbas_lti.util import transform_part_hierarchy
+from numbas_lti.util import transform_part_hierarchy, add_query_param
 import re
 import simplejson
 
@@ -235,7 +235,44 @@ class BrokenAttemptException(Exception):
     def __init__(self,attempt):
         self.attempt = attempt
 
-class RunAttemptView(RequireLockdownAppMixin, generic.detail.DetailView):
+class AttemptAccessMixin:
+    def dispatch(self, request, *args, **kwargs):
+        self.mode = self.get_mode()
+        return super().dispatch(request, *args, **kwargs)
+
+    def get_mode(self):
+        attempt = self.get_object()
+
+        if attempt.user != self.request.user:
+            user_data = attempt.resource.user_data(self.request.user)
+            if (user_data is not None and user_data.is_instructor) or request_is_instructor(self.request):
+                return 'review'
+            else:
+                raise PermissionDenied(gettext("You're not allowed to review this attempt."))
+
+        if attempt.completed():
+            if not attempt.review_allowed():
+                if attempt.resource.allow_review_from:
+                    template = get_template('numbas_lti/review_not_allowed.html')
+                    raise PermissionDenied(template.render({'allow_review_from': attempt.resource.allow_review_from}))
+                else:
+                    raise PermissionDenied(_("You're not allowed to review this attempt."))
+            return 'review'
+        else:
+            return 'normal'
+
+
+    def get_at_time(self):
+        if not request_is_instructor(self.request):
+            return
+
+        at_time = self.request.GET.get('at_time')
+        if at_time is not None:
+            at_time = datetime.datetime.fromisoformat(at_time)
+        return at_time
+
+
+class RunAttemptView(RequireLockdownAppMixin, AttemptAccessMixin, generic.detail.DetailView):
     model = Attempt
     context_object_name = 'attempt'
 
@@ -248,18 +285,58 @@ def get(self, request, *args, **kwargs):
             response = http.HttpResponseServerError(_("This attempt is broken - there isn't enough saved SCORM data to resume it."))
             self.mode = 'broken'
 
-        AttemptLaunch.objects.create(
-            attempt = self.object,
-            mode = self.mode,
-            user = self.request.user if not self.request.user.is_anonymous else None
-        )
         return response
 
     def get_context_data(self,*args,**kwargs):
         context = super(RunAttemptView,self).get_context_data(*args,**kwargs)
 
         attempt = self.get_object()
 
+        context['mode'] = self.mode
+
+        user = attempt.user
+        available_from, available_until = attempt.resource.available_for_user(user)
+
+        context['support_name'] = getattr(settings,'SUPPORT_NAME',None)
+        context['support_url'] = getattr(settings,'SUPPORT_URL',None)
+
+        context['available_until'] = available_until
+
+        context['load_cmi'] = True
+
+        at_time = self.get_at_time()
+        cmi_url = reverse('run_attempt_scorm_cmi', args=(attempt.pk,))
+        if at_time is not None:
+            cmi_url = add_query_param(cmi_url, {'at_time': at_time.isoformat()})
+
+        context['js_vars'] = {
+            'cmi_url': cmi_url,
+            'exam_url': attempt.exam.extracted_url+'/index.html',
+            'scorm_api_data': {
+                'show_attempts_url': reverse('show_attempts'),
+                'attempt_pk': attempt.pk,
+                'fallback_url': reverse('attempt_scorm_data_fallback', args=(attempt.pk,)),
+                'show_attempts_url': reverse('show_attempts'),
+                'allow_review_from': attempt.resource.allow_review_from.isoformat() if attempt.resource.allow_review_from else None,
+                'available_from': available_from.isoformat() if available_from else None,
+                'available_until': available_until.isoformat() if available_until else None,
+            }
+        }
+
+        return context
+
+class AttemptScormCMIView(JSONView, AttemptAccessMixin, generic.detail.DetailView):
+    download = False
+    model = Attempt
+
+    def get_data(self):
+        attempt = self.get_object()
+
+        scorm_cmi = attempt.scorm_cmi(at_time=self.get_at_time())
+
+        user = attempt.user
+        mode = self.get_mode()
+
         try:
             completion_status = attempt.scormelements.current('cmi.completion_status')
         except ScormElement.DoesNotExist:
@@ -287,31 +364,6 @@ def get_context_data(self,*args,**kwargs):
             entry = 'ab-initio'
             context['attempt'] = attempt
 
-
-        if attempt.completed():
-            mode = 'review'
-        else:
-            mode = 'normal'
-
-        if attempt.user != self.request.user:
-            user_data = attempt.resource.user_data(self.request.user)
-            if (user_data is not None and user_data.is_instructor) or request_is_instructor(self.request):
-                mode = 'review'
-            else:
-                raise PermissionDenied(gettext("You're not allowed to review this attempt."))
-
-        context['mode'] = self.mode = mode
-
-        user = attempt.user
-        available_from, available_until = attempt.resource.available_for_user(user)
-
-        at_time = self.request.GET.get('at_time')
-        if at_time is not None:
-            at_time = datetime.datetime.fromisoformat(at_time)
-
-        scorm_cmi = attempt.scorm_cmi(at_time=at_time)
-
-
         duration_extension_amount, duration_extension_units = attempt.resource.duration_extension_for_user(user)
         dynamic_cmi = {
             'cmi.mode': mode,
@@ -325,24 +377,20 @@ def get_context_data(self,*args,**kwargs):
         dynamic_cmi = {k: {'value':v,'time':now} for k,v in dynamic_cmi.items()}
         scorm_cmi.update(dynamic_cmi)
 
-        context['support_name'] = getattr(settings,'SUPPORT_NAME',None)
-        context['support_url'] = getattr(settings,'SUPPORT_URL',None)
-
-        context['scorm_cmi'] = simplejson.encoder.JSONEncoderForHTML().encode(scorm_cmi)
-        context['available_until'] = available_until
+        AttemptLaunch.objects.create(
+            attempt = attempt,
+            user = self.request.user if not self.request.user.is_anonymous else None,
+            mode = mode
+        )
 
-        context['js_vars'] = {
-            'exam_url': attempt.exam.extracted_url+'/index.html',
-            'scorm_cmi': scorm_cmi,
-            'attempt_pk': attempt.pk,
-            'fallback_url': reverse('attempt_scorm_data_fallback', args=(attempt.pk,)),
-            'show_attempts_url': reverse('show_attempts'),
-            'allow_review_from': attempt.resource.allow_review_from.isoformat() if attempt.resource.allow_review_from else None,
-            'available_from': available_from.isoformat() if available_from else None,
-            'available_until': available_until.isoformat() if available_until else None,
-        }
+        return scorm_cmi
 
-        return context
+    def render_to_response(self, context, **kwargs):
+        response = super().render_to_response(context, **kwargs)
+        response['Cache-Control'] = 'no-cache, no-store, must-revalidate'
+        response['Pragma'] = 'no-cache'
+        response['Expires'] = '0'
+        return response
 
 @require_POST
 def scorm_data_fallback(request,pk,*args,**kwargs):

numbas_lti/views/generic.py

@@ -51,12 +51,16 @@ def post(self, request, *args, **kwargs):
         return HttpResponseRedirect(self.get_success_url())
 
 class JSONView(object):
+    download = True
+
     def get_data(self):
         raise NotImplementedError()
+
     def get_filename(self):
         raise NotImplementedError()
 
     def render_to_response(self,context,**kwargs):
         response = JsonResponse(self.get_data())
-        response['Content-Disposition'] = 'attachment; filename="{}"'.format(self.get_filename())
+        if self.download:
+            response['Content-Disposition'] = 'attachment; filename="{}"'.format(self.get_filename())
         return response