Skip to content
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.

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

fix: Show default data on student scoreboard #1315

Merged
merged 2 commits into from
Mar 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
97 changes: 46 additions & 51 deletions game/tests/test_scoreboard.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
from __future__ import unicode_literals

from builtins import map, object, range
from builtins import map, range
from datetime import datetime, timedelta

from common.models import Class, Teacher, Student
from common.tests.utils.classes import create_class_directly
from common.tests.utils.organisation import create_organisation_directly
from common.tests.utils.student import create_school_student_directly
from common.tests.utils.student import (
create_school_student_directly,
create_independent_student_directly,
)
from common.tests.utils.teacher import signup_teacher_directly
from django.test import Client, TestCase
from django.urls import reverse
from hamcrest import *

from game.models import Attempt, Level, Episode
from game.views.scoreboard import StudentRow, scoreboard_data, Headers, StudentInTrouble
Expand All @@ -21,7 +23,7 @@ class ScoreboardTestCase(TestCase):
def test_teacher_multiple_students_multiple_levels(self):
episode_ids = [1]
episode1 = Episode.objects.get(id=1)
level_ids = [f"{x}" for x in range(1, len(episode1.levels)+1)]
level_ids = [f"{x}" for x in range(1, len(episode1.levels) + 1)]
level1 = Level.objects.get(name="1")
level2 = Level.objects.get(name="2")

Expand Down Expand Up @@ -75,24 +77,26 @@ def test_scoreboard_loads(self):
create_organisation_directly(email)
klass, name, access_code = create_class_directly(email)
create_school_student_directly(access_code)

url = reverse("scoreboard")
c = Client()
c.login(username=email, password=password)
response = c.get(url)
self.assertEqual(200, response.status_code)

assert response.status_code == 200

def test_student_can_see_classes(self):
"""A student should be able to see the classes they are in"""
mr_teacher = Teacher.objects.factory(
"Normal", "Teacher", "normal@school.edu", "secretpa$sword"
)
klass1, name1, access_code1 = create_class_directly(
klass, name1, _ = create_class_directly(
mr_teacher.user.user.email, class_name="Class 1"
)
klass2, name2, access_code2 = create_class_directly(
_, name2, _ = create_class_directly(
mr_teacher.user.user.email, class_name="Class 2"
)
student = Student.objects.schoolFactory(klass1, "some student", "secret")
student = Student.objects.schoolFactory(klass, "some student", "secret")

c = Client()
c.force_login(student.user.user)
Expand All @@ -103,9 +107,9 @@ def test_student_can_see_classes(self):
choices_in_form = [
v for (k, v) in response.context["form"]["classes"].field.choices
]
assert_that(choices_in_form, contains("Class 1"))
assert_that(choices_in_form, not_(contains("Class 2")))
assert_that(choices_in_form, has_length(1))
assert name1 in choices_in_form
assert name2 not in choices_in_form
assert len(choices_in_form) == 1

def test_admin_teacher_can_see_all_classes(self):
"""An admin should be able to see all classes, not just the ones they teach"""
Expand All @@ -119,13 +123,13 @@ def test_admin_teacher_can_see_all_classes(self):
admin_teacher.is_admin = True
admin_teacher.save()

klass1, name1, access_code1 = create_class_directly(
_, name1, _ = create_class_directly(
admin_teacher.user.user.email, class_name="Class 1"
)
klass2, name2, access_code2 = create_class_directly(
_, name2, _ = create_class_directly(
admin_teacher.user.user.email, class_name="Class 2"
)
klass3, name3, access_code3 = create_class_directly(
_, name3, _ = create_class_directly(
normal_teacher.user.user.email, class_name="Class 3"
)

Expand All @@ -138,9 +142,10 @@ def test_admin_teacher_can_see_all_classes(self):
choices_in_form = [
v for (k, v) in response.context["form"]["classes"].field.choices
]
assert_that(
choices_in_form, contains_inanyorder("Class 1", "Class 2", "Class 3")
)

assert name1 in choices_in_form
assert name2 in choices_in_form
assert name3 in choices_in_form

def test_non_admin_teacher_can_only_see_their_own_classes(self):
"""A teacher who is not an admin should only be able to see their classes, not ones taught by others"""
Expand All @@ -151,13 +156,13 @@ def test_non_admin_teacher_can_only_see_their_own_classes(self):
"Second", "Teacher", "admin@school.edu", "secretpa$sword2"
)

klass1, name1, access_code1 = create_class_directly(
_, name1, _ = create_class_directly(
teacher2.user.user.email, class_name="Class 1"
)
klass2, name2, access_code2 = create_class_directly(
_, name2, _ = create_class_directly(
teacher2.user.user.email, class_name="Class 2"
)
klass3, name3, access_code3 = create_class_directly(
_, name3, _ = create_class_directly(
teacher1.user.user.email, class_name="Class 3"
)

Expand All @@ -172,9 +177,10 @@ def test_non_admin_teacher_can_only_see_their_own_classes(self):
v for (k, v) in response.context["form"]["classes"].field.choices
]

assert_that(choices_in_form, contains("Class 3"))
assert_that(choices_in_form, not_(contains_inanyorder("Class 1", "Class 2")))
assert_that(choices_in_form, has_length(1))
assert name3 in choices_in_form
assert name1 not in choices_in_form
assert name2 not in choices_in_form
assert len(choices_in_form) == 1

# Other teacher logs in. Should see Class 1 and Class 2
c.login(username="admin@school.edu", password="secretpa$sword2")
Expand All @@ -184,9 +190,23 @@ def test_non_admin_teacher_can_only_see_their_own_classes(self):
v for (k, v) in response.context["form"]["classes"].field.choices
]

assert_that(choices_in_form, not_(contains("Class 3")))
assert_that(choices_in_form, contains_inanyorder("Class 1", "Class 2"))
assert_that(choices_in_form, has_length(2))
assert name3 not in choices_in_form
assert name1 in choices_in_form
assert name2 in choices_in_form
assert len(choices_in_form) == 2

def test_independent_student_cannot_see_scoreboard(self):
username, password, _ = create_independent_student_directly()

c = Client()
c.login(username=username, password=password)

url = reverse("scoreboard")
response = c.get(url)

assert "Scoreboard is only visible to school students and teachers" in str(
response.content
)


class ScoreboardCsvTestCase(TestCase):
Expand Down Expand Up @@ -286,31 +306,6 @@ def actual_data(self, content):
return header, rows


class MockStudent(object):
def __init__(self, student):
self.student = student

def is_student(self):
return True

def is_teacher(self):
return False

def is_independent_student(self):
return False


class MockTeacher(object):
def is_student(self):
return False

def is_teacher(self):
return True

def is_independent_student(self):
return False


def create_attempt(student, level, score):
attempt = Attempt.objects.create(
finish_time=datetime.fromtimestamp(1435305072),
Expand Down
18 changes: 13 additions & 5 deletions game/views/scoreboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,17 +200,23 @@ def scoreboard(request):

user = User(request.user.userprofile)
users_classes = classes_for(user)
all_episode_ids = list(range(1, 12))

if user.is_independent_student():
return render_no_permission_error(request)

if request.POST:
class_ids = set(map(int, request.POST.getlist("classes")))
episode_ids = set(map(int, request.POST.getlist("episodes")))
else:
# Show no data on page load
# Show no data on page load by default (if teacher)
class_ids = ()
episode_ids = ()

if user.is_independent_student():
return render_no_permission_error(request)
# If user is a student, show data for their class and all episodes
if user.is_student():
class_ids = {user.student.class_field.id}
episode_ids = set(all_episode_ids)

if is_teacher_with_no_classes_assigned(user, users_classes):
return render_no_permission_error(request)
Expand All @@ -232,7 +238,7 @@ def scoreboard(request):

all_levels = []

for episode_id in range(1, 12):
for episode_id in all_episode_ids:
episode = Episode.objects.get(id=episode_id)
all_levels += episode.levels

Expand Down Expand Up @@ -368,4 +374,6 @@ def is_teacher(self):
return hasattr(self.profile, "teacher")

def is_independent_student(self):
return hasattr(self.profile, "student") and self.student.is_independent()
return (
hasattr(self.profile, "student") and self.profile.student.is_independent()
)