Added testcases and a UserFactory, Django version bump to 1.11, fixes

and closes #23. Other minor changes.

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,

core/migrations/0016_auto_20170714_1634.py

+# -*- 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'),
+        ),
+    ]

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(

core/tests.py

+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())

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'),

core/views/index.py

-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'

core/views/login.py

 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')

requirements.txt

-Django~=1.10.6
+Django~=1.11.3
 django-extensions~=1.7.7
 djangorestframework~=3.6.3
 django_compressor~=2.1.1

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 ], ..., [...] ]

util/importer.py

+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