Skip to content

Add submision conversion feature for individual submission and by module #991

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
muhammadfaiz12 wants to merge 1 commit into from
Closed

Add submision conversion feature for individual submission and by module #991

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions deviations/forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ class DeadlineRuleDeviationForm(BaseDeviationForm):
initial=True,
label=_('LABEL_WITHOUT_LATE_PENALTY'),
)
without_late_submission_approval = forms.BooleanField(
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would name this field without negation: "approve_late_submissions", initial True. Negation is harder to understand for the users whereas enabling something is clear.

required=False,
initial=False,
label=_('LABEL_WITHOUT_LATE_APPROVAL'),
)

def __init__(self, *args: Any, **kwargs: Any) -> None:
super().__init__(*args, **kwargs)
Expand All @@ -114,6 +119,11 @@ class MaxSubmissionRuleDeviationForm(BaseDeviationForm):
label=_('LABEL_EXTRA_SUBMISSIONS'),
)

without_unofficial_submission_approval = forms.BooleanField(
required=False,
initial=False,
label=_('LABEL_WITHOUT_UNOFFICIAL_APPROVAL'),
)
def __init__(self, *args, **kwargs) -> None:
super().__init__(*args, **kwargs)
self.fields['module'].help_text = _('DEVIATION_MODULE_ADD_HELPTEXT')
Expand Down
46 changes: 46 additions & 0 deletions deviations/viewbase.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from itertools import groupby
from lib2to3.pytree import convert
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Mar 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this import unused? Remove this line.

from typing import Any, Dict, Iterable, List, Optional, Tuple, Type

from django.db import models
Expand All @@ -8,6 +9,10 @@
from django.shortcuts import get_object_or_404
from django.utils.text import format_lazy
from django.utils.translation import ugettext_lazy as _, ngettext
from django.utils import timezone
from django.utils.dateparse import parse_datetime
import datetime
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Mar 31, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The correct coding style is to place standard imports (built-in Python modules) before the third-party libraries. You can move import datetime to the top above all other imports.



from course.models import CourseModule, UserTag
from course.viewbase import CourseInstanceMixin, CourseInstanceBaseView
Expand All @@ -16,6 +21,9 @@
from authorization.permissions import ACCESS
from exercise.models import BaseExercise
from userprofile.models import UserProfile
from exercise.submission_models import Submission

from .forms import DeadlineRuleDeviationForm, MaxSubmissionRuleDeviationForm


class ListDeviationsView(CourseInstanceBaseView):
Expand Down Expand Up @@ -48,7 +56,12 @@ def form_valid(self, form: forms.BaseForm) -> HttpResponse:
exercise__in=exercises,
submitter__in=submitters,
)
if isinstance(form, DeadlineRuleDeviationForm ) and not form.cleaned_data.get("without_late_submission_approval"):
approve_late_submissions(submitters, exercises, form.cleaned_data)

elif isinstance(form, MaxSubmissionRuleDeviationForm ) and not form.cleaned_data.get("without_unofficial_submission_approval"):
approve_unofficial_submissions(submitters, exercises, form.cleaned_data)

Comment on lines +59 to +64
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. It's better to avoid isinstance when it is possible. In this case, the child view classes of AddDeviationsView in views.py could easily run the approval for either late or unofficial submissions. The child classes are specific to either deadline or submission attempt deviations. In the child class, you don't need isinstance.
  2. This approves submissions even if the deviation is cancelled in the override view (if existing_deviations: below). If the deviation is not updated at all, then you can't approve the submissions.

if existing_deviations:
# Some deviations already existed. Use OverrideDeviationsView to
# confirm which ones the user wants to override. Store the form
Expand Down Expand Up @@ -297,3 +310,36 @@ def get_submitters(form_data: Dict[str, Any]) -> models.QuerySet[UserProfile]:
models.Q(id__in=form_data.get('submitter', []))
| models.Q(taggings__tag__in=form_data.get('submitter_tag', []))
)

def approve_late_submissions(submitters, exercises, form_data):
minutes = form_data.get('minutes')
new_date = form_data.get('new_date')
for submitter in submitters:
for exercise in exercises:
submissions = exercise.get_submissions_for_student(submitter, exclude_errors=True)
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These database queries could be made more efficient. Now, this fetches also normal, graded submissions that couldn't be approved. There could be many such submissions, but they are not needed here.

Note that late submissions do not always have any late penalty and thus, it is not enough to check submissions with submission.late_penalty_applied is not None. If the module does not allow late submissions, but the exercise category allows unofficial submissions, then late submissions become unofficial without any late penalty (submission.late_penalty_applied is None).

If the teacher adds deviations to many exercises and students at once, then making database queries separately for each exercise and student pair might be needlessly slow. Making hundreds of database queries for a single HTTP request is never good. It is possible to fetch more data in a single query.

for submission in submissions:
new_deadline = None
if new_date:
string_date = str(new_date)[:16]
new_deadline = timezone.make_aware(
parse_datetime(string_date),
timezone.get_current_timezone())
else:
new_deadline = submission.submission_time + datetime.timedelta(minutes=minutes)

if submission.late_penalty_applied is not None and submission.submission_time <= new_deadline:
submission.convert_penalized_submission()
submission.save()

def approve_unofficial_submissions(submitters, exercises, form_data):
extra_submissions = form_data.get('extra_submissions')
for submitter in submitters:
for exercise in exercises:
converted_counter = extra_submissions
submissions_unordered = exercise.get_submissions_for_student(submitter, exclude_errors=True)
submissions = [submission for submission in reversed(submissions_unordered)] #reverse the order to get older submission first
for submission in submissions:
if submission.status == Submission.STATUS.UNOFFICIAL and converted_counter > 0:
submission.convert_penalized_submission()
submission.save()
converted_counter -= 1
72 changes: 72 additions & 0 deletions exercise/staff_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,14 @@
from authorization.permissions import ACCESS
from course.viewbase import CourseInstanceBaseView, CourseInstanceMixin
from course.models import (
CourseModule,
Enrollment,
USERTAG_EXTERNAL,
USERTAG_INTERNAL,
)
from deviations.models import MaxSubmissionsRuleDeviation
from exercise.cache.points import CachedPoints
from exercise.exercise_models import BaseExercise
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

from .exercise_models import BaseExercise was already done a few lines below.

from lib.helpers import settings_text, extract_form_errors
from lib.viewbase import BaseRedirectView, BaseFormView, BaseView
from notification.models import Notification
Expand Down Expand Up @@ -530,3 +532,73 @@ def form_valid(self, form):
def form_invalid(self, form):
messages.error(self.request, _('FAILURE_SAVING_CHANGES'))
return super().form_invalid(form)

class SubmissionConversionView(SubmissionMixin, BaseRedirectView):
"""
A POST-only view that updates a student's late or unofficial submission
to normal submission. Changed the status and remove the penalty
"""
def post(self, request: HttpRequest, *args: Any, **kwargs: Any) -> HttpResponse:
Copy link
Copy Markdown
Contributor

@PasiSa PasiSa Feb 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this missing access_mode = ACCESS.ASSISTANT?

Copy link
Copy Markdown
Contributor Author

@muhammadfaiz12 muhammadfaiz12 Feb 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, i'm not sure, but if you think i should add that then i could do that

Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it should be ACCESS.GRADING so that it takes the setting exercise.allow_assistant_grading True/False into account.

self.submission = self.get_submission_object()
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The super classes already defined self.submission, so there is no need to re-assign self.submission here.

self.submission.convert_penalized_submission()
self.submission.save()
return self.redirect(self.submission.get_inspect_url())


class SubmissionConversionByModuleView(CourseInstanceMixin, BaseRedirectView):
"""
A POST-only view that by module bulks updates a student's late or unofficial submission
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo bulks

to normal submission. Changed the status and remove the penalty
"""

user_kw = 'user_id'
module_kw = 'module_id'
access_mode = ACCESS.ASSISTANT
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to also check exercise.allow_assistant_grading separately.


def get_course_module_object(self):
return get_object_or_404(
CourseModule,
id=self.kwargs[self.module_kw],
course_instance=self.instance
)
Comment on lines +558 to +563
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method isn't used anywhere in this class. Fetching the course module in get_resource_objects() is good.


def get_resource_objects(self):
self.kwargs[self.user_kw] = self.request.POST[self.user_kw],
self.kwargs[self.user_kw] = self.kwargs[self.user_kw][0]
Comment on lines +566 to +567
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trailing comma should be removed from the first line and then, the second line is not needed. The comma turns the value into a tuple.

Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.kwargs is set automatically by Django from the URL variables captured in URLconf regular expressions. It is unconventional to assign POST data there. Note that you could use self.request.POST directly in other methods too.

self.kwargs[self.module_kw] = self.request.POST[self.module_kw]
super().get_resource_objects()

#getting module and user by id.
self.module = get_object_or_404(
CourseModule,
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad indentation.

id=self.kwargs[self.module_kw])

self.student = get_object_or_404(
User,
id=self.kwargs[self.user_kw],
)

def post(self, request: HttpRequest, *args: Any, **kwargs: Any) -> HttpResponse:
approve_scope = self.request.POST["approve-scope"]
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.request.POST.get('approve-scope') is safer so that the server does not crash to KeyError whenever the user manipulates the HTML form or POST data and removes some hidden field.

approve_type = self.request.POST["approve-type"]
is_scope_exercise = approve_scope == "single-exercise"
exercise_id = self.request.POST["exercise_id"]
is_late = False if approve_type == "isUnofficial" else True
is_unofficial = False if approve_type == "isLate" else True
Comment on lines +586 to +587
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
is_late = False if approve_type == "isUnofficial" else True
is_unofficial = False if approve_type == "isLate" else True
is_late = approve_type != "isUnofficial"
is_unofficial = approve_type != "isLate"


profile = self.student.userprofile
exercises = []
if is_scope_exercise:
exercises = BaseExercise.objects.filter(id=exercise_id)
else:
exercises = BaseExercise.objects.filter(course_module=self.module)

for exercise in exercises:
submissions = exercise.get_submissions_for_student(self.student.userprofile, exclude_errors=True)
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The database queries can be made more efficient. It is not necessary to iterate over all the normal submissions.

Note that if a course module does not allow late submissions, but unofficial submissions are allowed in the exercise category, then late submissions become unofficial without any late penalty. Though, since the user interface has choices "late only" or "unofficial only", then "late only" should imply that it does not pick any unofficial submissions. Note that unofficial submissions may have late penalty set when the module allows late submissions.

for submission in submissions:
if ((is_unofficial and submission.status == Submission.STATUS.UNOFFICIAL) or (is_late and submission.late_penalty_applied is not None)):
submission.convert_penalized_submission()
submission.save()

link = reverse('user-results', kwargs={'user_id': profile.id, **self.instance.get_url_kwargs()})
return self.redirect(link)
14 changes: 12 additions & 2 deletions exercise/submission_models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,16 @@ def clean_post_parameters(self):
del self._files
del self._data

def convert_penalized_submission(self):
"""
Removes penalty and Sets the points for this submission object based on original score.
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will rephrase this docstring. The official PEP Python styleguides require the imperative mood ("Remove the penalty").

Then this will set the status of submission to ready

The method is used to convert late or unofficial submission to fully graded one.
"""
self.set_points(self.service_points, self.service_max_points, no_penalties=True)
self.set_ready(convert_operation=True)

def set_points(self, points, max_points, no_penalties=False):
"""
Sets the points and maximum points for this submissions. If the given
Expand Down Expand Up @@ -467,9 +477,9 @@ def scale_grade_to(self, percentage):
def set_waiting(self):
self.status = self.STATUS.WAITING

def set_ready(self):
def set_ready(self, convert_operation=False):
self.grading_time = timezone.now()
if self.status != self.STATUS.UNOFFICIAL or self.force_exercise_points:
if (self.status != self.STATUS.UNOFFICIAL or self.force_exercise_points) or convert_operation:
self.status = self.STATUS.READY

# Fire set hooks.
Expand Down
5 changes: 5 additions & 0 deletions exercise/templates/exercise/_user_results.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,11 @@ <h3 class="panel-title">

{% points_progress module %}
{{ module.introduction|safe }}

{% if module.late_allowed and module.late_percent > 0 %}

{% endif %}

Comment on lines +88 to +92
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this empty if here?

</div>
{% if not exercise_accessible and not is_course_staff %}
<div class="alert alert-warning clearfix site-message">
Expand Down
19 changes: 18 additions & 1 deletion exercise/templates/exercise/staff/_assessment_panel.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,24 @@
{% endif %}
</div>
<div>
<button
<form method="post" style="display: inline-block">
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This form is unnecessary. You have other form elements inside the modal dialog.

{% csrf_token %}
<button
data-toggle="modal"
data-target="#submission-approval-modal"
class="aplus-button--secondary aplus-button--sm aplus-button--left"
type="button"
>
{% translate 'APPROVE_SUBMISSION' %}
</button><button
data-toggle="modal"
data-target="#approval-help-modal"
class="aplus-button--secondary aplus-button--sm aplus-button--right"
type="button"
title="{% translate 'HELP' %}"
>?</button>
Comment on lines +31 to +37
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The separate help modal dialog is unnecessary. The help text can be moved to the approval modal.

</form>
<button
data-toggle="modal"
data-target="#details-modal"
class="aplus-button--secondary aplus-button--sm"
Expand Down
21 changes: 21 additions & 0 deletions exercise/templates/exercise/staff/_late_submission_approval_help_modal.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{% load i18n %}
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's delete this file since the help text can be moved to the actual approval modal.

{% load course %}

<div class="modal" id="approval-help-modal" tabindex="-1" role="dialog" aria-labelledby="approval-help-modal-label">
<div class="modal-dialog" role="document">
<div class="modal-content">
<div class="modal-header">
<button type="button" class="close" data-dismiss="modal" aria-label="{% translate "CLOSE" %}">
<span aria-hidden="true">&times;</span>
</button>
<h4 class="modal-title" id="approval-help-modal-label">
{% translate "SUBMISSION_APPROVAL_HELP" %}
</h4>
</div>

<div class="modal-body">
{% translate "SUBMISSION_APPROVAL_DESCRIPTION" %}
</div>
</div>
</div>
</div>
79 changes: 79 additions & 0 deletions exercise/templates/exercise/staff/_late_submission_approval_modal.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
{% load i18n %}
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our style guide expects indentation with TAB in HTML files. This file has mixed tab and space indentations.

{% load course %}

<div class="modal" id="submission-approval-modal" tabindex="-1" role="dialog" aria-labelledby="resubmit-modal-label">
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aria-labelledby has incorrect value. This was clearly copied from the regrading modal. This needs to be changed for this modal.

<div class="modal-dialog" role="document">
<div class="modal-content">
<div class="modal-header">
<button type="button" class="close" data-dismiss="modal" aria-label="{% translate "CLOSE" %}">
<span aria-hidden="true">&times;</span>
</button>
<h4 class="modal-title" id="resubmit-modal-label">
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This id should match aria-labelledby in the top. This value was copied from the regrading modal and needs to be changed.

{% translate 'LATE_SUBMISSION_APPROVAL_CONFIRMATION_HEADER' %}
</h4>
</div>

<div class="modal-body">
<p>{% translate 'LATE_SUBMISSION_APPROVAL_CONFIRMATION_TEXT' %}</p>
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes the translated messages have keys like "LATE_SUBMISSION" even though they do not apply to only late submissions. Unofficial submissions are included too.

<p>{{submission.submitters.all.0.name_with_student_id}}</p>
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inspect view already adds a template variable submitter, thus it should be used here instead of querying the database.

<dl>
<dt>{% translate 'LATE_SUBMISSION_APPROVAL_CONFIRMATION_TEXT_DETAIL' %}</dt>
<dd>Module: {{ module.name|parse_localization }}</dd>
<dd>Exercise: {{ exercise.name|parse_localization }}</dd>
<dd>Submission ID & Time: {{ submission.id }} - {{ submission.submission_time }}</dd>
</dl>
<br>
<button onclick="OpenMultipleConversionform();" name = "approve-multiple" class="aplus-button--secondary aplus-button--xs">{% translate 'LATE_SUBMISSION_APPROVAL_APPROVE_MULTIPLE_BUTTON' %}</button>
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onclick is not considered the best practice. Let's write a simple script that finds the button element with its id and attaches the event handler in JS.

<br>
<br>
<form method="post" id="convert-module-form" action="{% url 'submission-conversion-module' course_slug=course.url instance_slug=instance.url %}" style="display: none">
{% csrf_token %}
<input type="hidden" id="module_id" name="module_id" value="{{ module.id }}">
<input type="hidden" id="user_id" name="user_id" value="{{ submission.submitters.all.0.id }}">
<input type="hidden" id="exercise_id" name="exercise_id" value="{{ exercise.id }}">
<p>{% translate 'LATE_SUBMISSION_APPROVAL_APPROVE_MULTIPLE_SELECT_SCOPE' %}</p>
<input type="radio" id="single-exercise" name="approve-scope" value="single-exercise">
<label for="single-exercise">{% translate 'LATE_SUBMISSION_APPROVAL_APPROVE_MULTIPLE_SCOPE_EXERCISE' %}</label><br>
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The label's for attribute is not needed if the input element is wrapped inside the label element.

<input type="radio" id="whole-module" name="approve-scope" value="whole-module">
<label for="whole-module">{% translate 'LATE_SUBMISSION_APPROVAL_APPROVE_MULTIPLE_SCOPE_MODULE' %}</label><br>

<br><p>{% translate 'LATE_SUBMISSION_APPROVAL_APPROVE_MULTIPLE_SELECT_TYPE' %}</p>
<input type="radio" id="isLate" name="approve-type" value="isLate">
<label for="isLate"> {% translate 'LATE_SUBMISSION_APPROVAL_APPROVE_MULTIPLE_TYPE_LATE' %}</label><br>
<input type="radio" id="isUnofficial" name="approve-type" value="isUnofficial">
<label for="isUnofficial"> {% translate 'LATE_SUBMISSION_APPROVAL_APPROVE_MULTIPLE_TYPE_UNOFFICIAL' %}</label><br>
<input type="radio" id="isAll" name="approve-type" value="isAll">
<label for="isAll"> {% translate 'LATE_SUBMISSION_APPROVAL_APPROVE_MULTIPLE_TYPE_ALL' %}</label><br>
<button
class="aplus-button--default aplus-button--sm"
type="submit"
>
{% translate 'APPROVE_MULTIPLE_SUBMISSIONS' %}
</button>
</form>
<form method="post" id="convert-singular-form" action="{{ submission|url:'submission-conversion' }}" style="display: inline-block">
{% csrf_token %}
<button
class="aplus-button--default aplus-button--sm"
type="submit"
>
{% translate 'APPROVE_SUBMISSION' %}
</button>
</form>
</div>
</div>
</div>
</div>
<script>
function OpenMultipleConversionform(){
moduleform = document.getElementById('convert-module-form')
singularform = document.getElementById('convert-singular-form')
Comment on lines +69 to +70
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In JavaScript, if you don't declare variables with var, let or const, they become global and pollute the global namespace. That is bad.

if (moduleform.style.display === "none") {
document.getElementById('convert-module-form').style.display = 'inline-block';
document.getElementById('convert-singular-form').style.display = 'none';
} else {
document.getElementById('convert-module-form').style.display = 'none';
document.getElementById('convert-singular-form').style.display = 'inline-block';
}
}
</script>
2 changes: 2 additions & 0 deletions exercise/templates/exercise/staff/inspect_submission.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@
</div>
{% include "exercise/staff/_submission_data_modal.html" %}
{% include "exercise/staff/_resubmit_modal.html" %}
{% include "exercise/staff/_late_submission_approval_modal.html" %}
{% include "exercise/staff/_late_submission_approval_help_modal.html" %}
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The separate help modal is deleted since the help text can be moved to the actual approval modal.

{% endblock %}

{% block scripts %}
Expand Down
7 changes: 7 additions & 0 deletions exercise/tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,13 @@ def test_submission_late_penalty_applied(self):
deviation.save()
self.late_late_submission_when_late_allowed.set_points(5, 10)
self.assertAlmostEqual(self.late_late_submission_when_late_allowed.late_penalty_applied, 0.2)

def test_submission_late_conversion(self):
convert_submission_url = self.late_submission.get_url('submission-conversion')
response = self.client.get(convert_submission_url)
self.assertEqual(response.status_code, 302)
self.assertTrue(self.late_submission.late_penalty_applied is None)

Comment on lines +575 to +580
Copy link
Copy Markdown
Contributor

@markkuriekkinen markkuriekkinen Apr 1, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would make sense to add more tests than just this. For example, there could be unit tests that directly call the method Submission.convert_penalized_submission() without making HTTP requests with self.client.


def test_early_submission(self):
self.course_module_with_late_submissions_allowed.opening_time = self.tomorrow
Expand Down
Loading