From 8a22d7ce80460e3b41fc2218ad63e6d546fb597f Mon Sep 17 00:00:00 2001 From: Christian Lawson-Perfect Date: Fri, 12 Jul 2024 14:08:11 +0100 Subject: [PATCH] starting work on due date see #315 The Resource model now has a field `allow_student_reopen`, which determines whether students can re-open their own attempts. Students can re-open their own attempts if the resource is still available, and the resource allows it. --- numbas_lti/forms.py | 23 +++++++- numbas_lti/models.py | 57 ++++++++++++++----- numbas_lti/static/api.js | 31 +++++++++- numbas_lti/static/numbas_lti.css | 5 ++ .../numbas_lti/management/attempts.html | 7 ++- .../templates/numbas_lti/show_attempts.html | 32 ++++++++++- numbas_lti/util.py | 7 ++- numbas_lti/views/attempt.py | 52 ++++++++++++----- 8 files changed, 180 insertions(+), 34 deletions(-) diff --git a/numbas_lti/forms.py b/numbas_lti/forms.py index fd6e5b56..d8252c51 100644 --- a/numbas_lti/forms.py +++ b/numbas_lti/forms.py @@ -121,17 +121,34 @@ def fieldsets(self): class ResourceSettingsForm(FieldsetFormMixin, ModelForm): class Meta: model = Resource - fields = ['grading_method','include_incomplete_attempts','max_attempts','show_marks_when','report_mark_time','allow_review_from','available_from','available_until','email_receipts','require_lockdown_app', 'lockdown_app_password', 'seb_settings', 'show_lockdown_app_password'] + fields = [ + 'grading_method', + 'include_incomplete_attempts', + 'max_attempts', + 'show_marks_when', + 'report_mark_time', + 'allow_review_from', + 'allow_student_reopen', + 'available_from', + 'due_date', + 'available_until', + 'email_receipts', + 'require_lockdown_app', + 'lockdown_app_password', + 'seb_settings', + 'show_lockdown_app_password' + ] fieldsets = [ (_('Grading'), ('grading_method', 'include_incomplete_attempts',)), - (_('Attempts'), ('max_attempts',)), + (_('Attempts'), ('max_attempts', 'allow_student_reopen',)), (_('Feedback'), ('show_marks_when', 'report_mark_time', 'allow_review_from', 'email_receipts',)), - (_('Availability'), ('available_from', 'available_until')), + (_('Availability'), ('available_from', 'due_date', 'available_until')), (_('Lockdown app'), ('require_lockdown_app', 'lockdown_app_password', 'seb_settings', 'show_lockdown_app_password')), ] widgets = { 'allow_review_from': DateTimeInput(format=datetime_format), 'available_from': DateTimeInput(format=datetime_format), + 'due_date': DateTimeInput(format=datetime_format), 'available_until': DateTimeInput(format=datetime_format), 'lockdown_app_password': forms.TextInput(attrs={'placeholder': getattr(settings,'LOCKDOWN_APP',{}).get('password','')}), } diff --git a/numbas_lti/models.py b/numbas_lti/models.py index 0ad9adbf..5d276bbd 100644 --- a/numbas_lti/models.py +++ b/numbas_lti/models.py @@ -1,6 +1,7 @@ from asgiref.sync import async_to_sync from channels.layers import get_channel_layer from collections import defaultdict +from dataclasses import dataclass from django.conf import settings from django.contrib.auth.models import User from django.core import signing @@ -34,6 +35,7 @@ import re import shutil import time +from typing import Optional import uuid from zipfile import ZipFile @@ -41,7 +43,7 @@ from .groups import group_for_attempt, group_for_resource_stats, group_for_resource from .report_outcome import report_outcome, report_outcome_for_attempt, ReportOutcomeException from .diff import make_diff, apply_diff -from .util import parse_scorm_timeinterval +from .util import parse_scorm_timeinterval, iso_time class LineItemDoesNotExist(Exception): def __init__(self, resource): @@ -447,6 +449,15 @@ def get_absolute_url(self): ('unwanted', _('Not wanted')), ] +@dataclass +class Availability: + """ + A representation of the times when a resource is available. + """ + from_time: Optional[datetime] + until_time: Optional[datetime] + due_date: Optional[datetime] + class Resource(models.Model): exam = models.ForeignKey(Exam,blank=True,null=True,on_delete=models.SET_NULL,related_name='main_exam_of') title = models.CharField(max_length=300,default='') @@ -459,7 +470,9 @@ class Resource(models.Model): show_marks_when = models.CharField(max_length=20, default='always', choices=SHOW_SCORES_MODES, verbose_name=_('When to show scores to students')) available_from = models.DateTimeField(blank=True, null=True, verbose_name=_('Available from')) available_until = models.DateTimeField(blank=True, null=True, verbose_name=_('Available until')) + due_date = models.DateTimeField(blank=True, null=True, verbose_name=_('Due date')) allow_review_from = models.DateTimeField(blank=True, null=True, verbose_name=_('Allow students to review attempts from')) + allow_student_reopen = models.BooleanField(default=True, verbose_name=_('Allow students to re-open attempts while the resource is available?')) report_mark_time = models.CharField(max_length=20,choices=REPORT_TIMES,default='immediately',verbose_name=_('When to report scores back')) email_receipts = models.BooleanField(default=False,verbose_name=_('Email attempt receipts to students on completion?')) @@ -546,6 +559,7 @@ def students(self): def available_for_user(self,user=None): afrom = self.available_from auntil = self.available_until + due_date = self.due_date deadline_extension = timedelta(0) if user is not None: @@ -558,8 +572,13 @@ def available_for_user(self,user=None): afrom = change.available_from if change.available_until is not None: auntil = change.available_until + if change.due_date is not None: + due_date = change.due_date - return (afrom, auntil + deadline_extension if auntil is not None else None) + if auntil is not None: + auntil += deadline_extension + + return Availability(from_time=afrom, until_time=auntil, due_date=self.due_date) def duration_extension_for_user(self, user): duration = 0 @@ -582,15 +601,17 @@ def duration_disabled_for_user(self, user): return self.access_changes.for_user(user).filter(disable_duration=True).exists() def availability_json(self,user=None): - available_from, available_until = self.available_for_user(user) + availability = self.available_for_user(user) + if user is not None: extension_amount, extension_units = self.duration_extension_for_user(user) else: extension_amount, extension_units = None, None data = { - 'available_from': available_from.isoformat() if available_from else None, - 'available_until': available_until.isoformat() if available_until else None, - 'allow_review_from': self.allow_review_from.isoformat() if self.allow_review_from else None, + 'available_from': iso_time(availability.from_time), + 'available_until': iso_time(availability.until_time), + 'due_date': iso_time(availability.due_date), + 'allow_review_from': iso_time(self.allow_review_from), 'duration_extension': { 'amount': extension_amount, 'units': extension_units, @@ -603,20 +624,20 @@ def is_available(self,user=None): if user is not None and user.is_anonymous: return False - available_from, available_until = self.available_for_user(user) + availability = self.available_for_user(user) - if available_from is None and available_until is None: + if availability.from_time is None and availability.until_time is None: return True now = timezone.now() available = False - if available_from is None or available_until is None: - available = (available_from is None or now >= available_from) and (available_until is None or now<=available_until) - elif available_from < available_until: - available = available_from <= now <= available_until + if availability.from_time is None or availability.until_time is None: + available = (availability.from_time is None or now >= availability.from_time) and (availability.until_time is None or now<=availability.until_time) + elif availability.from_time < availability.until_time: + available = availability.from_time <= now <= availability.until_time else: - available = now <= available_until or now >= available_from + available = now <= availability.until_time or now >= availability.from_time return available @@ -928,6 +949,7 @@ class AccessChange(models.Model): resource = models.ForeignKey(Resource,on_delete=models.CASCADE,related_name='access_changes') available_from = models.DateTimeField(blank=True, null=True, verbose_name=_('Available from')) available_until = models.DateTimeField(blank=True, null=True, verbose_name=_('Available until')) + due_date = models.DateTimeField(blank=True, null=True, verbose_name=_('Due date')) extend_deadline = models.DurationField(blank=True, null=True, verbose_name=_('Extend deadline by')) max_attempts = models.PositiveIntegerField(blank=True, null=True, verbose_name=_('Maximum attempts per user'), help_text=_('Zero means unlimited attempts.')) extend_duration = models.FloatField(blank=True, null=True, verbose_name=_('Extend exam duration by')) @@ -1265,6 +1287,15 @@ def completed(self): return True return self.completion_status=='completed' + def student_can_reopen(self): + if not self.completed(): + return False + + if not self.resource.is_available(self.user): + return False + + return self.resource.allow_student_reopen + def finalise(self): if self.end_time is None: self.end_time = timezone.now() diff --git a/numbas_lti/static/api.js b/numbas_lti/static/api.js index 29ed78c7..5d4a17df 100644 --- a/numbas_lti/static/api.js +++ b/numbas_lti/static/api.js @@ -25,6 +25,10 @@ function SCORM_API(options) { this.fallback_url = options.fallback_url; this.show_attempts_url = options.show_attempts_url; + /** The time that this launch of the attempt started. + */ + this.session_start_time = new Date(); + /** Key to save data under in localStorage */ this.localstorage_key = 'numbas-lti-attempt-'+this.attempt_pk+'-scorm-data'; @@ -111,6 +115,10 @@ SCORM_API.prototype = { */ last_error: 0, + /** Was this attempt launched after the due date? + */ + launched_after_due_date: false, + /** Update the availability dates for the resource */ update_availability_dates: function(data,first) { @@ -118,9 +126,12 @@ SCORM_API.prototype = { this.allow_review_from = load_date(data.allow_review_from); var available_from = load_date(data.available_from); var available_until = load_date(data.available_until); + var due_date = load_date(data.due_date); var changed = !(dates_equal(available_from, this.available_from) && dates_equal(available_until, this.available_until)); this.available_from = available_from; this.available_until = available_until; + this.due_date = due_date; + this.launched_after_due_date = data.launched_after_due_date; var unavailable_time = this.unavailable_time(); if(unavailable_time) { @@ -299,6 +310,7 @@ SCORM_API.prototype = { /** Force the exam to end. */ end: function(reason) { + this.SetValue('cmi.completion_status','completed'); if(reason!==undefined) { this.SetValue('x.reason ended',reason); } @@ -381,6 +393,13 @@ SCORM_API.prototype = { if(!this.is_available()) { this.end('not available'); } + + var now = get_now(); + // Close the attempt when the due date passes, if the attempt wasn't launched after the due date. + if(this.due_date !== undefined && !this.launched_after_due_date && now >= this.due_date) { + this.end('due date passed'); + } + } }, @@ -439,12 +458,21 @@ SCORM_API.prototype = { /** The time that the attempt becomes unavailable. */ unavailable_time: function() { - if(this.offline || this.available_until === undefined) { + if(this.offline) { + return; + } + + if(this.due_date !== undefined && !this.launched_after_due_date) { + return this.due_date; + } + + if(this.available_until === undefined) { return; } if(this.available_from===undefined || this.available_from < this.available_until) { return this.available_until; } else { + // available_from is after available_until, so this resource is available for all time except the interval between those times. return; } }, @@ -458,6 +486,7 @@ SCORM_API.prototype = { return true; } var now = get_now(); + if(this.available_from===undefined || this.available_until===undefined) { return (this.available_from===undefined || now >= this.available_from) && (this.available_until===undefined || now <= this.available_until); } diff --git a/numbas_lti/static/numbas_lti.css b/numbas_lti/static/numbas_lti.css index 970a52b2..e0360419 100644 --- a/numbas_lti/static/numbas_lti.css +++ b/numbas_lti/static/numbas_lti.css @@ -211,6 +211,7 @@ a.button:not(:hover, :focus) { padding: var(--double-space); margin: var(--quad-space) 0; border: thin solid var(--alert-color); + max-width: max-content; } .alert > :is(h1,h2,h3,h4,h5,h6) { @@ -593,6 +594,10 @@ body.manage-consumer .period-name { top: 3em; } +body.show-attempts table.attempts form { + display: inline; +} + .card-list { display: flex; flex-wrap: wrap; diff --git a/numbas_lti/templates/numbas_lti/management/attempts.html b/numbas_lti/templates/numbas_lti/management/attempts.html index 6b6089cb..2a8ee9b4 100644 --- a/numbas_lti/templates/numbas_lti/management/attempts.html +++ b/numbas_lti/templates/numbas_lti/management/attempts.html @@ -88,7 +88,12 @@

{% translate "Attempts" %}

{% translate "Broken" %} {% else %} {{attempt.get_completion_status_display}} - {% if attempt.completed %}{% icon 'eye-open' %} {% translate "Reopen" %}{% endif %} + {% if attempt.completed %} +
+ {% csrf_token %} + +
+ {% endif %} {% endif %} diff --git a/numbas_lti/templates/numbas_lti/show_attempts.html b/numbas_lti/templates/numbas_lti/show_attempts.html index 559e6733..4bc0beac 100644 --- a/numbas_lti/templates/numbas_lti/show_attempts.html +++ b/numbas_lti/templates/numbas_lti/show_attempts.html @@ -7,6 +7,8 @@ {% block title %}{{resource.title}} - {{block.super}}{% endblock title %} +{% block body_class %}show-attempts {{block.super}}{% endblock %} + {% block content %}

@@ -28,13 +30,25 @@

{% endif %} + {% if is_available %} + {% if due_date_passed %} +
+

{% blocktranslate %}The due date for this activity has passed, but it is still open. You can continue to attempt this activity, subject to any late work policy.{% endblocktranslate %}

+
+ {% endif %} + {% else %} +
+

{% blocktranslate %}This activity is now closed.{% endblocktranslate %}{% if object_list.exists %} {% blocktranslate %}You can review your existing attempts.{% endblocktranslate %}{% endif %}

+
+ {% endif %} + {% for message in messages %}
{{message}}
{% endfor %} - +
@@ -74,6 +88,16 @@

{% icon 'play' %} {% translate "Review this attempt" %} + + {% if attempt.student_can_reopen %} +
+ {% csrf_token %} + + + {% endif %} {% else %} {% if attempt.resume_allowed %} @@ -88,7 +112,11 @@

{% translate "Start time" %}
{% if resource.show_marks_when == 'review' and resource.allow_review_from %} -

{% blocktranslate with time_iso=resource.allow_review_from|date:"c" time=resource.allow_review_from %}Full review will be available from .{% endblocktranslate %}

+ {% if review_allowed %} +

{% blocktranslate %}Full review is now allowed.{% endblocktranslate %}

+ {% else %} +

{% blocktranslate with time_iso=resource.allow_review_from|date:"c" time=resource.allow_review_from %}Full review will be available from .{% endblocktranslate %}

+ {% endif %} {% endif %}

{% blocktranslate %}These are the feedback settings for this activity:{% endblocktranslate %}

diff --git a/numbas_lti/util.py b/numbas_lti/util.py index 6cbf4bbc..f6233800 100644 --- a/numbas_lti/util.py +++ b/numbas_lti/util.py @@ -1,6 +1,6 @@ import string import re -from datetime import timedelta +from datetime import timedelta, datetime from urllib.parse import urlparse, urlunparse, parse_qs, urlencode from django.http import QueryDict @@ -86,3 +86,8 @@ def add_query_param(url,extras): ) return url +def iso_time(time: datetime) -> str: + """ + Convert a datetime to an ISO format string if it's not None, otherwise return None. + """ + return time.isoformat() if time else None diff --git a/numbas_lti/views/attempt.py b/numbas_lti/views/attempt.py index 92376ea8..7780a2f4 100644 --- a/numbas_lti/views/attempt.py +++ b/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, add_query_param +from numbas_lti.util import transform_part_hierarchy, add_query_param, iso_time import re import simplejson @@ -94,10 +94,17 @@ def post(self, request, *args, **kwargs): return self.get(request,*args,**kwargs) -class ReopenAttemptView(MustBeInstructorMixin,generic.detail.DetailView): +class ReopenAttemptView(MustBeInstructorMixin,generic.edit.UpdateView): model = Attempt - def get(self, request, *args, **kwargs): + def check_allowed(self, request): + attempt = self.get_object() + if self.request.user == attempt.user and attempt.student_can_reopen(): + return True + + return super().check_allowed(request) + + def post(self, request, *args, **kwargs): attempt = self.get_object() e = ScormElement.objects.create( attempt=attempt, @@ -106,8 +113,14 @@ def get(self, request, *args, **kwargs): time=timezone.now(), counter=1 ) - messages.add_message(self.request,messages.SUCCESS,_('{}\'s attempt has been reopened.'.format(attempt.user.get_full_name()))) - return redirect(self.reverse_with_lti('manage_attempts',args=(attempt.resource.pk,))) + + if self.request.user != attempt.user: + messages.add_message(self.request,messages.SUCCESS,_('{}\'s attempt has been reopened.'.format(attempt.user.get_full_name()))) + next_url = self.reverse_with_lti('manage_attempts',args=(attempt.resource.pk,)) + else: + next_url = self.reverse_with_lti('show_attempts') + + return redirect(next_url) class AttemptSCORMListing(MustHaveExamMixin,MustBeInstructorMixin,ResourceManagementViewMixin,generic.detail.DetailView): model = Attempt @@ -188,10 +201,10 @@ def dispatch(self,request,*args,**kwargs): resource = request.resource if not resource.is_available(request.user): now = timezone.now() - available_from, available_until = resource.available_for_user(request.user) - if available_from is not None and now < available_from: + availability = resource.available_for_user(request.user) + if availability.from_time is not None and now < availability.from_time: template = get_template('numbas_lti/not_available_yet.html') - raise PermissionDenied(template.render({'available_from': available_from})) + raise PermissionDenied(template.render({'available_from': availability.from_time})) return new_attempt(request) else: @@ -201,10 +214,17 @@ def get_context_data(self,*args,**kwargs): context = super(ShowAttemptsView,self).get_context_data(*args,**kwargs) resource = self.request.resource + user = self.request.user + + availability = resource.available_for_user(user) + now = timezone.now() context['resource'] = resource - context['can_start_new_attempt'] = self.request.resource.can_start_new_attempt(self.request.user) + context['can_start_new_attempt'] = resource.can_start_new_attempt(user) + context['due_date_passed'] = availability.due_date is not None and now >= availability.due_date + context['is_available'] = resource.is_available(user) context['exam_info'] = resource.exam.get_feedback_settings(completed=False,review_allowed=False) + context['review_allowed'] = resource.show_marks_when in ('always', 'completed') or (resource.show_marks_when == 'review' and (resource.allow_review_from is None or now >= resource.allow_review_from)) return context @@ -292,12 +312,13 @@ def get_context_data(self,*args,**kwargs): context['mode'] = self.mode user = attempt.user - available_from, available_until = attempt.resource.available_for_user(user) + availability = 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['available_until'] = availability.until_time + context['due_date'] = availability.due_date context['load_cmi'] = True @@ -306,6 +327,9 @@ def get_context_data(self,*args,**kwargs): if at_time is not None: cmi_url = add_query_param(cmi_url, {'at_time': at_time.isoformat()}) + now = timezone.now() + launched_after_due_date = availability.due_date is not None and now > availability.due_date + context['js_vars'] = { 'cmi_url': cmi_url, 'exam_url': attempt.exam.extracted_url+'/index.html', @@ -315,8 +339,10 @@ def get_context_data(self,*args,**kwargs): 'fallback_url': self.reverse_with_lti('attempt_scorm_data_fallback', args=(attempt.pk,)), 'show_attempts_url': self.reverse_with_lti('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, + 'available_from': iso_time(availability.from_time), + 'available_until': iso_time(availability.until_time), + 'due_date': iso_time(availability.due_date), + 'launched_after_due_date': launched_after_due_date, } }