From d9a8c68e742c6a6a77d22b1a181f167f60169a24 Mon Sep 17 00:00:00 2001
From: Florian Aucomte <33633200+faucomte97@users.noreply.github.com>
Date: Tue, 21 Feb 2023 14:13:20 +0000
Subject: [PATCH] fix: Custom levels always have max score of 10 (#1422)
---
Pipfile.lock | 30 ++++++------
game/forms.py | 6 +--
game/templates/game/scoreboard.html | 4 +-
game/tests/test_scoreboard.py | 73 +++++++++++++++++++++++------
game/views/scoreboard.py | 21 ++++++---
5 files changed, 93 insertions(+), 41 deletions(-)
diff --git a/Pipfile.lock b/Pipfile.lock
index 5aeffdcac..953edc30d 100644
--- a/Pipfile.lock
+++ b/Pipfile.lock
@@ -342,11 +342,11 @@
},
"setuptools": {
"hashes": [
- "sha256:95f00380ef2ffa41d9bba85d95b27689d923c93dfbafed4aecd7cf988a25e012",
- "sha256:bb6d8e508de562768f2027902929f8523932fcd1fb784e6d573d2cafac995a48"
+ "sha256:9d3de8591bd6f6522594406fa46a6418eabd0562dacb267f8556675762801514",
+ "sha256:ed4e75fafe103c79b692f217158ba87edf38d31004b9dbc1913debb48793c828"
],
"markers": "python_version >= '3.7'",
- "version": "==67.3.2"
+ "version": "==67.3.3"
},
"sqlparse": {
"hashes": [
@@ -412,10 +412,10 @@
"develop": {
"aimmo": {
"hashes": [
- "sha256:87785b3474b4374c2bf392122447c02c63c8cf2650749d84f2e9d02d1fc42f52",
- "sha256:f4c6c55fcfc2656f7962524f99bdcc4a0380e54d7d8eeef257052cf92820d501"
+ "sha256:3aea2cdb7c314161dbb4c37228543bafb9f79dbb88dca925713a6bf549227e04",
+ "sha256:f263588ac4a3ead1176a7144f3d7a87e614c15b6a0ed14ad1811c14d13cff8e6"
],
- "version": "==2.5.13"
+ "version": "==2.5.15"
},
"asgiref": {
"hashes": [
@@ -666,11 +666,11 @@
},
"google-auth": {
"hashes": [
- "sha256:5045648c821fb72384cdc0e82cc326df195f113a33049d9b62b74589243d2acc",
- "sha256:ed7057a101af1146f0554a769930ac9de506aeca4fd5af6543ebe791851a9fbd"
+ "sha256:5fd170986bce6bfd7bb5c845c4b8362edb1e0cba901e062196e83f8bb5d5d32c",
+ "sha256:75d76ea857df65938e1f71dcbcd7d0cd48e3f80b34b8870ba229c9292081f7ef"
],
"markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3, 3.4, 3.5'",
- "version": "==2.16.0"
+ "version": "==2.16.1"
},
"greenlet": {
"hashes": [
@@ -1149,11 +1149,11 @@
},
"setuptools": {
"hashes": [
- "sha256:95f00380ef2ffa41d9bba85d95b27689d923c93dfbafed4aecd7cf988a25e012",
- "sha256:bb6d8e508de562768f2027902929f8523932fcd1fb784e6d573d2cafac995a48"
+ "sha256:9d3de8591bd6f6522594406fa46a6418eabd0562dacb267f8556675762801514",
+ "sha256:ed4e75fafe103c79b692f217158ba87edf38d31004b9dbc1913debb48793c828"
],
"markers": "python_version >= '3.7'",
- "version": "==67.3.2"
+ "version": "==67.3.3"
},
"six": {
"hashes": [
@@ -1241,11 +1241,11 @@
},
"zipp": {
"hashes": [
- "sha256:23f70e964bc11a34cef175bc90ba2914e1e4545ea1e3e2f67c079671883f9cb6",
- "sha256:e8b2a36ea17df80ffe9e2c4fda3f693c3dad6df1697d3cd3af232db680950b0b"
+ "sha256:188834565033387710d046e3fe96acfc9b5e86cbca7f39ff69cf21a4128198b7",
+ "sha256:9e5421e176ef5ab4c0ad896624e87a7b2f07aca746c9b2aa305952800cb8eecb"
],
"markers": "python_version >= '3.7'",
- "version": "==3.13.0"
+ "version": "==3.14.0"
}
}
}
diff --git a/game/forms.py b/game/forms.py
index e3ed145ef..ddb13049e 100644
--- a/game/forms.py
+++ b/game/forms.py
@@ -17,10 +17,8 @@ def __init__(self, *args, **kwargs):
)
# Each tuple in choices has two elements, id and name of each level
# First element is the actual value set on the model
- # Second element is the string displayed on the drop down menu
- episodes_choices = (
- (episode.id, episode.name) for episode in Episode.objects.all()
- )
+ # Second element is the string displayed on the dropdown menu
+ episodes_choices = ((episode.id, episode.name) for episode in Episode.objects.all())
self.fields["episodes"] = forms.MultipleChoiceField(
choices=itertools.chain(episodes_choices),
widget=forms.CheckboxSelectMultiple(),
diff --git a/game/templates/game/scoreboard.html b/game/templates/game/scoreboard.html
index f0b05f4e7..f42acb98f 100644
--- a/game/templates/game/scoreboard.html
+++ b/game/templates/game/scoreboard.html
@@ -181,9 +181,9 @@ Scoreboard
{% endif %}
{% endfor %}
{% if user|is_logged_in_as_teacher %}
- {% if student.percentage_complete > 75 %}
+ {% if student.success_rate > 75 %}
- {% elif student.percentage_complete < 33 %}
+ {% elif student.success_rate < 33 %}
|
{% else %}
|
diff --git a/game/tests/test_scoreboard.py b/game/tests/test_scoreboard.py
index 1f8a56198..a2288fb36 100644
--- a/game/tests/test_scoreboard.py
+++ b/game/tests/test_scoreboard.py
@@ -13,25 +13,41 @@
from game.models import Attempt, Level, Episode
from game.tests.utils.level import create_save_level
-from game.views.scoreboard import StudentRow, scoreboard_data, Headers, StudentInTrouble, shared_level_to_name
+from game.views.scoreboard import (
+ StudentRow,
+ scoreboard_data,
+ Headers,
+ StudentInTrouble,
+ shared_level_to_name,
+ shared_levels_data,
+ SharedHeaders,
+)
from game.views.scoreboard_csv import scoreboard_csv, Headers as CSVHeaders, SharedHeaders as CSVSharedHeaders
class ScoreboardTestCase(TestCase):
def test_teacher_multiple_students_multiple_levels(self):
- episode_ids = [1]
+ # Setup official levels data
+ episode_ids = [1, 2]
episode1 = Episode.objects.get(id=1)
- level_ids = [f"{x}" for x in range(1, len(episode1.levels) + 1)]
+ episode2 = Episode.objects.get(id=2)
+ level_ids = [f"{x}" for x in range(1, len(episode1.levels) + len(episode2.levels) + 1)]
level1 = Level.objects.get(name="1")
- level2 = Level.objects.get(name="2")
+ level13 = Level.objects.get(name="13")
clas, student, student2 = set_up_data()
create_attempt(student, level1, 20)
create_attempt(student2, level1, 2)
- create_attempt(student2, level2, 16)
+ create_attempt(student2, level13, 16)
- all_levels = [level1, level2]
+ # Setup custom levels data
+ shared_level = create_save_level(student, "custom_level1", shared_with=[student2.new_user])
+
+ create_attempt(student2, shared_level, 10)
+
+ all_levels = [level1, level13]
+ all_shared_levels = [shared_level]
attempts_per_student = {
student: Attempt.objects.filter(level__in=all_levels, student=student, is_best_attempt=True).select_related(
@@ -42,8 +58,19 @@ def test_teacher_multiple_students_multiple_levels(self):
).select_related("level"),
}
+ shared_attempts_per_student = {
+ student2: Attempt.objects.filter(
+ level__in=all_shared_levels, student=student2, is_best_attempt=True
+ ).select_related("level"),
+ }
+
+ # Generate results
student_data, headers, level_headers, levels_sorted = scoreboard_data(episode_ids, attempts_per_student)
+ shared_headers, shared_level_headers, shared_student_data = shared_levels_data(
+ student.new_user, all_shared_levels, shared_attempts_per_student
+ )
+ # Check data for official levels matches
assert headers == Headers
assert level_headers == [f"L{id}" for id in level_ids]
@@ -54,7 +81,7 @@ def test_teacher_multiple_students_multiple_levels(self):
assert student_row.name == student.user.user.first_name
assert student_row.total_score == 20
assert student_row.total_time == timedelta(0)
- assert student_row.level_scores[1]["score"] == 20
+ assert student_row.level_scores[all_levels[0].id]["score"] == 20
assert student_row.completed == 1
assert student_row.success_rate == 100.0
@@ -63,13 +90,28 @@ def test_teacher_multiple_students_multiple_levels(self):
assert student_row.name == student2.user.user.first_name
assert student_row.total_score == 18
assert student_row.total_time == timedelta(0)
- assert student_row.level_scores[1]["score"] == 2
- assert student_row.level_scores[2]["score"] == 16
+ assert student_row.level_scores[all_levels[0].id]["score"] == 2
+ assert student_row.level_scores[all_levels[1].id]["score"] == 16
assert student_row.completed == 1
assert (
student_row.success_rate == 45.0
) ## the scores, (2 + 16 = 18), divided by the total possible, (2 * 20 = 40), 18/40 = 45%
+ # Check data for custom levels matches
+ assert shared_headers == SharedHeaders
+ assert shared_level_headers == [f"{shared_level.name} ({shared_level.owner})"]
+
+ assert len(shared_student_data) == 1
+
+ student_row = shared_student_data[0]
+ assert student_row.class_field.name == clas.name
+ assert student_row.name == student2.user.user.first_name
+ assert student_row.total_score == 10
+ assert student_row.total_time == timedelta(0)
+ assert student_row.level_scores[shared_level.id]["score"] == 10
+ assert student_row.completed == 1
+ assert student_row.success_rate == 100.0
+
def test_scoreboard_loads(self):
email, password = signup_teacher_directly()
create_organisation_directly(email)
@@ -211,9 +253,12 @@ def test_scoreboard_csv(self):
response = scoreboard_csv(student_rows, levels, improvement_data, shared_levels_headers, shared_level_rows)
# Gather the data from the CSV
- actual_scoreboard_header, actual_scoreboard_rows, actual_shared_levels_header, actual_shared_levels_rows = self.actual_data(
- response.content.decode("utf-8"), len(students)
- )
+ (
+ actual_scoreboard_header,
+ actual_scoreboard_rows,
+ actual_shared_levels_header,
+ actual_shared_levels_rows,
+ ) = self.actual_data(response.content.decode("utf-8"), len(students))
# Check the headers and the number or rows match expectations
assert actual_scoreboard_header == self.expected_scoreboard_header(levels)
@@ -281,7 +326,7 @@ def student_row(self, class_name=None):
total_score=total_score,
level_scores=level_scores,
completed=2,
- percentage_complete=45,
+ success_rate=45,
)
return row, student
@@ -312,7 +357,7 @@ def shared_student_row(self, student, shared_levels):
total_score=0,
level_scores=level_scores,
completed=0,
- percentage_complete=0,
+ success_rate=0,
)
return row
diff --git a/game/views/scoreboard.py b/game/views/scoreboard.py
index f8910c976..d33c34df5 100644
--- a/game/views/scoreboard.py
+++ b/game/views/scoreboard.py
@@ -65,12 +65,21 @@ def student_row(levels_sorted, student, best_attempts):
if attempt:
max_score = 0
- if not attempt.level.disable_route_score:
- max_score += 10
- if not attempt.level.disable_algorithm_score:
- max_score += 10
- if int(attempt.level.name) < 13:
- max_score = 20
+
+ # Max score logic as follows:
+ # - All custom levels have a max score of 10
+ # - All official levels have a max score of 20 if both route score and algorithm scores are enabled
+ # except for levels 1-12 which have a max score of 20 even though the algorithm score is disabled
+ if attempt.level.episode is None:
+ max_score = 10
+ else:
+ if not attempt.level.disable_route_score:
+ max_score += 10
+ if not attempt.level.disable_algorithm_score:
+ max_score += 10
+ if int(attempt.level.name) < 13:
+ max_score = 20
+
num_all += 1
if attempt.score:
if attempt.score / max_score >= threshold:
|