diff --git a/core/fixtures/testdata-core.json b/core/fixtures/testdata-core.json index fa913e9d82a430eb60e4042eb63a026da7ed8115..28b9bea58d4a332e87c9e0c4b7dc97402a89daea 100644 --- a/core/fixtures/testdata-core.json +++ b/core/fixtures/testdata-core.json @@ -3,9 +3,9 @@ "fields": { "full_score": 10, "name": "Aufgabe 01", - "possible_solution": "solution", + "solution": "solution", "slug": "brezmaphgocfuikw", - "task_description": "description" + "description": "description" }, "model": "core.submissiontype", "pk": 1 @@ -14,9 +14,9 @@ "fields": { "full_score": 20, "name": "Aufgabe 02", - "possible_solution": "solution", + "solution": "solution", "slug": "zbjfwldsuhqgxvmn", - "task_description": "description" + "description": "description" }, "model": "core.submissiontype", "pk": 2 @@ -43,7 +43,6 @@ }, { "fields": { - "pre_corrections": "COMPILER", "seen_by_student": false, "slug": "qgleatcwzfxsdnjr", "student": 1, @@ -55,7 +54,6 @@ }, { "fields": { - "pre_corrections": "LINKER ERROR", "seen_by_student": false, "slug": "mrthqgsloaydjfnc", "student": 1, @@ -67,7 +65,6 @@ }, { "fields": { - "pre_corrections": "ALL GOOD", "seen_by_student": false, "slug": "hunkgevtcfdobyxw", "student": 2, @@ -79,7 +76,6 @@ }, { "fields": { - "pre_corrections": "QUACK", "seen_by_student": false, "slug": "gurvbyzxjfmhdiep", "student": 2, diff --git a/core/migrations/0016_auto_20170714_1634.py b/core/migrations/0016_auto_20170714_1634.py new file mode 100644 index 0000000000000000000000000000000000000000..227a18a80fc0cbc9515d367fa32fcfe20a2555d3 --- /dev/null +++ b/core/migrations/0016_auto_20170714_1634.py @@ -0,0 +1,41 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.3 on 2017-07-14 16:34 +from __future__ import unicode_literals + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0015_auto_20170713_1220'), + ] + + operations = [ + migrations.RemoveField( + model_name='submission', + name='pre_corrections', + ), + migrations.AlterField( + model_name='feedback', + name='of_reviewer', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='reviewed_submissions', to=settings.AUTH_USER_MODEL), + ), + migrations.AlterField( + model_name='feedback', + name='of_tutor', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='feedback_list', to=settings.AUTH_USER_MODEL), + ), + migrations.AlterField( + model_name='student', + name='exam', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='students', to='core.ExamType'), + ), + migrations.AlterField( + model_name='submission', + name='type', + field=models.ForeignKey(on_delete=django.db.models.deletion.PROTECT, related_name='submissions', to='core.SubmissionType'), + ), + ] diff --git a/core/models.py b/core/models.py index 100e6f1a68a7428288024b80b62b0a03304e7be6..f9f2bcb0925e66f699f6eb4156f8b9cbbc387bfe 100644 --- a/core/models.py +++ b/core/models.py @@ -33,9 +33,9 @@ def random_matrikel_no() -> str: """Use as a default value for student's matriculation number. Returns: - str: an eight digit number that starts with a 2 + str: an eight digit number """ - return str(2000_0000 + randrange(1000_0000)) + return str(10_000_000 + randrange(90_000_000)) def get_annotated_tutor_list(): @@ -56,9 +56,8 @@ class ExamType(models.Model): belong to. The information is not needed and is currently, just used to detect if students already have enough points to pass an exam. - It is NOT - intended to use this for including different exams regarding submissions - types. + It is NOT intended to use this for including different exams regarding + submissions types. Attributes ---------- @@ -158,18 +157,21 @@ class Student(models.Model): Information like email (if given), and the username are stored in the associated user model. - Attributes - ---------- - exam : ForeignKey - Which module the student wants to be graded in - has_logged_in : BooleanField - Login is permitted once. If this is set the user can not log in. - matrikel_no : CharField - The matriculation number of the student - name : CharField - The students full real name - user : UserModel - The django auth user that makes a student authenticates with. + Attributes: + exam (ForeignKey): + Which module the student wants to be graded in + + has_logged_in (BooleanField): + Login is permitted once. If this is set the user can not log in. + + matrikel_no (CharField): + The matriculation number of the student + + name (CharField): + The students full real name + + user (UserModel): + The django auth user that makes a student authenticates with. """ has_logged_in = models.BooleanField(default=False) name = models.CharField(max_length=50, default="__no_name__") @@ -298,7 +300,6 @@ class Submission(models.Model): type : OneToOneField Relation to the type containing meta information """ - # Fields seen_by_student = models.BooleanField(default=False) text = models.TextField(blank=True) slug = models.SlugField( diff --git a/core/tests.py b/core/tests.py index 7ce503c2dd97ba78597f6ff6e4393132753573f6..5831a6a23c467e5ec1e814328fbb7d44d2911ac1 100644 --- a/core/tests.py +++ b/core/tests.py @@ -1,3 +1,69 @@ +from django.contrib.auth.models import Group, User from django.test import TestCase -# Create your tests here. +from core.models import Student, Submission, SubmissionType, Feedback +from util.importer import GradyUserFactory + + +def ensure_groups(): + Group.objects.get_or_create(name='Tutors') + Group.objects.get_or_create(name='Students') + Group.objects.get_or_create(name='Reviewers') + + +class FeedbackTestCase(TestCase): + + factory = GradyUserFactory() + + @classmethod + def setUpTestData(cls): + ensure_groups() + + def setUp(self): + self.tutor = self.factory.make_tutor() + self.student = self.factory.make_student() + + submission_type = SubmissionType.objects.create( + name='Cooking some crystal with Jesse') + Submission.objects.create(student=self.student, type=submission_type) + Submission.assign_tutor(self.tutor) + + def test_can_assign_tutor(self): + self.assertEqual(self.tutor.feedback_list.count(), 1) + + def test_feedback_origin_is_manual(self): + feedback = self.tutor.feedback_list.all()[0] + self.assertEqual(feedback.origin, Feedback.MANUAL) + + def test_feedback_status_is_editable(self): + feedback = self.tutor.feedback_list.all()[0] + self.assertEqual(feedback.status, Feedback.EDITABLE) + + +class FactoryTestCase(TestCase): + + factory = GradyUserFactory() + + @classmethod + def setUpTestData(cls): + ensure_groups() + + def test_make_student(self): + + student = self.factory.make_student() + + self.assertEqual(student.user.groups.filter(name='Students').count(), 1) + self.assertEqual(student.exam, None) + self.assertEqual(len(str(student.matrikel_no)), 8) + + def test_can_create_reviewer(self): + self.assertTrue(Group.objects.get(name='Reviewers') + in self.factory.make_reviewer().groups.all()) + + def test_can_create_tutor(self): + self.assertTrue(Group.objects.get(name='Tutors') + in self.factory.make_tutor().groups.all()) + + def test_can_create_student(self): + self.assertTrue(Group.objects.get(name='Students') + in self.factory.make_student().user.groups.all()) diff --git a/core/urls.py b/core/urls.py index 867d523a9e2629c36d74dcdda61c6a9514a50d8c..b79a568e167b1b441c76339261ed68171ec56176 100644 --- a/core/urls.py +++ b/core/urls.py @@ -4,9 +4,9 @@ from django.contrib.staticfiles.urls import staticfiles_urlpatterns from core import views urlpatterns = [ - url(r'^$', views.index, name='index'), - url(r'^login/$', views.user_login, name='login'), - url(r'^logout/$', views.user_logout, name='logout'), + url(r'^$', views.IndexView.as_view(), name='index'), + url(r'^login/$', views.Login.as_view(), name='login'), + url(r'^logout/$', views.Logout.as_view(), name='logout'), url(r'^start/$', views.user_home, name='start'), url(r'^feedback/create/$', views.create_feedback, name='CreateFeedback'), diff --git a/core/views/index.py b/core/views/index.py index 8c55022c3c84be1d0c2a0cb25f2d2b73bff9cce6..24fa020f9d7cdd698d48b20319976b9ecab68a96 100644 --- a/core/views/index.py +++ b/core/views/index.py @@ -1,5 +1,5 @@ -from django.shortcuts import render +from django.views.generic import TemplateView -def index(request): - return render(request, 'core/index.html') +class IndexView(TemplateView): + template_name = 'core/index.html' diff --git a/core/views/login.py b/core/views/login.py index 708a1a89a1f8e67add915f8541d86dbb64111e02..33243ef7a1b167e27b89f76e0514ed5d7b20f31b 100644 --- a/core/views/login.py +++ b/core/views/login.py @@ -1,48 +1,64 @@ from django.contrib import messages -from django.contrib.auth import authenticate, login, logout -from django.contrib.auth.decorators import login_required +from django.contrib.auth import authenticate, login +from django.contrib.auth.views import LoginView, LogoutView from django.http import HttpResponseRedirect -from django.urls import reverse +from django.urls import reverse, reverse_lazy from core.custom_annotations import in_groups -__all__ = ('user_login', 'user_logout') -def is_disabled(user): +def is_student(user): + return in_groups(user, ('Students',)) + +def is_deactivated_student(user) -> bool: + """Checks if the user is linked to a student and if the field user + has_logged_in is True or not. Obviously a superuser should not be a student. + + Args: + user (User object): The original user of some request. + + Returns: + bool: True if user is a student and has already logged in + """ + if user.is_superuser: + return False + if in_groups(user, ('Students',)): - if not user.student.has_logged_in: - user.student.disable() - return False - else: - return True - return False + return user.student.has_logged_in + +class Login(LoginView): -def user_login(request): + success_url = reverse_lazy('start') + template_name = 'core/index.html' - if request.method == 'POST': - username = request.POST['username'] - password = request.POST['password'] + def get(self, request): + return HttpResponseRedirect(reverse('index')) + + def form_valid(self, form): + username = form.cleaned_data['username'] + password = form.cleaned_data['password'] user = authenticate(username=username, password=password) - if user is not None: - if user.is_superuser or (user.is_active and not is_disabled(user)): - login(request, user) - return HttpResponseRedirect(reverse('start')) - else: - messages.warning(request, "Your Grady account is disabled.") - return HttpResponseRedirect(reverse('index')) + if user is not None and user.is_active and not is_deactivated_student(user): + login(self.request, user) + + # disable the user if s/he is a student + if is_student(user) and not user.is_superuser: + user.student.disable() - else: + print(self.get_success_url()) + return HttpResponseRedirect(self.get_success_url()) + + # Handle all the errors separately + if is_deactivated_student(user) or not user.is_active: + messages.warning(self.request, "Your Grady account has been deactivated.") + elif not user: # Bad login details were provided. So we can't log the user in. print("Invalid login details: {0}, {1}".format(username, password)) - messages.error(request, "Invalid login details supplied.") - return HttpResponseRedirect(reverse('index')) - else: - return HttpResponseRedirect(reverse('index')) + messages.error(self.request, "Invalid login details supplied.") + return super().form_invalid(form) -@login_required(login_url='/') -def user_logout(request): - logout(request) - return HttpResponseRedirect(reverse('index')) +class Logout(LogoutView): + next_page = reverse_lazy('index') diff --git a/requirements.txt b/requirements.txt index b6bb6dad38921a3b22dcd34bd9b20d0ea06c89d4..abd5c4c0ab810b6dcfa2e260a682252670385f63 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -Django~=1.10.6 +Django~=1.11.3 django-extensions~=1.7.7 djangorestframework~=3.6.3 django_compressor~=2.1.1 diff --git a/util/convert.py b/util/convert.py index 5798b2458bbc61f25700a90283b73efa379644c3..e038b7f39257b3661101332d117452cc6b87f392 100755 --- a/util/convert.py +++ b/util/convert.py @@ -67,10 +67,10 @@ task_head_re = re.compile(r'^Quellcode Frage(?P<title>.*) \d{8}$') # nor parsing the weird mat no matno_re = re.compile(r'^(?P<matrikel_no>\d{8})-(\d{3})-(\d{3})$') -# Modify these iterators in order to change extraction behaviour - def converter(infile, usernames=None, number_of_tasks=0,): + # Modify these iterators in order to change extraction behaviour + def sheet_iter_meta(sheet): """ yield first and second col entry as tuple of (name, matnr) """ for row in (sheet.row(i) for i in range(1, sheet.nrows)): @@ -89,6 +89,7 @@ def converter(infile, usernames=None, number_of_tasks=0,): # nice! name2mat = dict(sheet_iter_meta(meta)) + assert meta.nrows - 1 == len(name2mat), f'{meta.nrows} != {len(name2mat)}' # from xls to lists and namedtuples # [ [user0, task0_h, code0, ..., taskn, coden ], ..., [...] ] diff --git a/util/importer.py b/util/importer.py index 9b6226ec326e7367607ce16e6263b0c874820b40..36202ce8a35d645578fc2ea9a4fb246a5454787d 100644 --- a/util/importer.py +++ b/util/importer.py @@ -1,9 +1,9 @@ +import configparser import csv import json import os import readline import secrets -import configparser from typing import Callable from django.contrib.auth.models import Group, User @@ -15,9 +15,9 @@ from core.models import (ExamType, Feedback, Student, Submission, from util.messages import * from util.processing import EmptyTest -STUDENTS = Group.objects.get(name='Students') -TUTORS = Group.objects.get(name='Tutors') -REVIEWERS = Group.objects.get(name='Reviewers') +STUDENTS = 'Students' +TUTORS = 'Tutors' +REVIEWERS = 'Reviewers' HISTFILE = '.importer_history' RECORDS = '.importer' @@ -28,11 +28,11 @@ NO = 'y/N' valid = {"yes": True, "y": True, "ye": True, "no": False, "n": False} -FEEDBACK_MAPPER = { - util.processing.EmptyTest.__name__ : Feedback.WAS_EMPTY, - util.processing.CompileTest.__name__ : Feedback.DID_NOT_COMPILE, - util.processing.LinkTest.__name__ : Feedback.COULD_NOT_LINK, - util.processing.UnitTestTest.__name__ : Feedback.FAILED_UNIT_TESTS, +ORIGIN_ORDER = { + Feedback.WAS_EMPTY, + Feedback.DID_NOT_COMPILE, + Feedback.COULD_NOT_LINK, + Feedback.FAILED_UNIT_TESTS, } TEST_ORDER = ( @@ -42,6 +42,8 @@ TEST_ORDER = ( util.processing.UnitTestTest.__name__, ) +FEEDBACK_MAPPER = dict(zip(TEST_ORDER, ORIGIN_ORDER)) + class chdir_context(object): """ @@ -84,50 +86,104 @@ def i(prompt: str, default: str='', is_path: bool=False, is_file: bool=False): return answer -def store_password(username, group, password): +def store_password(username, groupname, password): storage = configparser.ConfigParser() storage.read(PASSWORDS) - if not group in storage: - storage[group] = {} + if not groupname in storage: + storage[groupname] = {} - storage[group][username] = password + storage[groupname][username] = password with open(PASSWORDS, 'w') as passwd_file: storage.write(passwd_file) +class GradyUserFactory: -def add_user(username: str, group: str, **kwargs): - """ This is a specific wrapper for the django update_or_create method of - objects. - * A new user is created and password and group are set accordingly - * If the user was there before password is NOT change but group is. A - user must only have one group. + def __init__(self, password_generator_func=get_xkcd_password, *args, **kwargs): + self.password_generator_func = password_generator_func - Args: - username (str): the username is the login name - group (str): the (only) group the user should belong to - **kwargs: more attributes for user creation + @staticmethod + def get_random_name(prefix='', suffix='', k=1): + return ''.join((prefix, get_xkcd_password(k), suffix)) - Returns: - TYPE: Description - """ - username = username.strip() + def make_default_user(self, username, **kwargs): + return User.objects.update_or_create(username=username, defaults=kwargs) - user, created = User.objects.update_or_create( - username=username, - defaults=kwargs - ) + def make_user_in_group(self, username, groupname, store_pw=False, **kwargs): + """ This is a specific wrapper for the django update_or_create method of + objects. + * A new user is created and password and group are set accordingly + * If the user was there before password is NOT change but group is. A + user must only have one group. + + Args: + username (str): the username is the login name + group (Group object): the (only) group the user should belong to + **kwargs: more attributes for user creation - if created: - password = get_xkcd_password() - user.set_password(password) + Returns: + (User object, str): The user object that was added to the group and + the password of that user if it was created. + """ + username = username.strip() + + user, created = self.make_default_user( + username=username, + **kwargs + ) + + if created: + password = self.password_generator_func() + user.set_password(password) + user.save() + + if created and store_pw: + store_password(username, group, password) + + group = Group.objects.get(name=groupname) + user.groups.clear() # remove all other groups + user.groups.add(group) user.save() - store_password(username, group.name, password) + return user + - user.groups.clear() # remove all other groups - group.user_set.add(user) + def make_user_in_student_group(self, username, **kwargs): + return self.make_user_in_group(username, STUDENTS, **kwargs) + + def make_student(self, username=None, name='__name', matrikel_no=None, exam=None, **kwargs): + if not username: + username = self.get_random_name(prefix='student_') + + user = self.make_user_in_student_group(username, **kwargs) + + student, _ = Student.objects.update_or_create( + name=name, + defaults={ + 'user': user, + 'exam': exam, + # TODO: find an elegant way to include optionals iff they exist + } + ) + + return student + + def make_tutor(self, username=None, **kwargs): + if not username: + username = self.get_random_name(prefix='tutor_') + return self.make_user_in_group(username, TUTORS, **kwargs) + + def make_reviewer(self, username=None, **kwargs): + if not username: + username = self.get_random_name(prefix='reviewer_') + return self.make_user_in_group(username, REVIEWERS, **kwargs) + + +def add_user(username, group, **kwargs): + user = GradyUserFactory().update_or_create_user_in_group( + username, group, store_pw=True, **kwargs + ) return user