refactoring of Take and Student, add CoursePlan

- A student is no longer associated to courses directly.
  Instead, he can define one or more course-plans, which can
  also be shared(made public)

- Implement object level auth for course plans - only the owner
  can access it, unless the plan was made public, then it can be read
  by all users(as well as unauthenticated ones), but it cannot be
  modified by them.

- Add tests for the new course plan serializer, as well as CoursePlan views and
  permissions

- Student.year_in_studies is now calculated based on his chosen track
  year*

- Student.remaining and Student.trickle down are temporarily disabled*

* Perhaps these calculations should be moved to be someone else's
responsibility, either into CoursePlan or a new API?

Author: nmdanny

Closure_Project/rest_api/admin.py

@@ -2,11 +2,10 @@
 
 # Register your models here.
 from django.contrib import admin
-from .models import Course, Student, CourseGroup, Track, Take
+from .models import Course, Student, CourseGroup, Track, Take, CoursePlan
 admin.site.register(Course)
 admin.site.register(Student)
 admin.site.register(CourseGroup)
 admin.site.register(Track)
 admin.site.register(Take)
-
-
+admin.site.register(CoursePlan)

Closure_Project/rest_api/migrations/0005_auto_20210828_2210.py

@@ -0,0 +1,43 @@
+# Generated by Django 3.2.4 on 2021-08-28 19:10
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('rest_api', '0004_auto_20210629_0256'),
+    ]
+
+    operations = [
+        migrations.RemoveField(
+            model_name='student',
+            name='courses',
+        ),
+        migrations.RemoveField(
+            model_name='student',
+            name='year_in_studies',
+        ),
+        migrations.RemoveField(
+            model_name='take',
+            name='student',
+        ),
+        migrations.CreateModel(
+            name='CoursePlan',
+            fields=[
+                ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('name', models.TextField(max_length=50, null=True)),
+                ('public', models.BooleanField(default=False)),
+                ('created_at', models.DateTimeField(auto_now_add=True)),
+                ('modified_at', models.DateTimeField(auto_now=True)),
+                ('owner', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='rest_api.student')),
+            ],
+        ),
+        migrations.AddField(
+            model_name='take',
+            name='course_plan',
+            field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='rest_api.courseplan'),
+            preserve_default=False,
+        ),
+    ]

Closure_Project/rest_api/models.py

@@ -1,7 +1,7 @@
 # Create your models here.
 import uuid
 from enum import Enum
-from typing import List
+from typing import List, Optional
 
 from django.core.exceptions import ValidationError
 
@@ -205,77 +205,87 @@ def __str__(self):
 class Student(models.Model):
     user = models.OneToOneField(User, on_delete=models.CASCADE)
     track = models.ForeignKey(Track, on_delete=models.CASCADE, null=True)
-    year_in_studies = models.IntegerField(choices=Year.choices, null=True)
-    courses = models.ManyToManyField(Course, through='Take', blank=True)
+    # courses = models.ManyToManyField(Course, through='Take', blank=True)
 
     def __str__(self):
         return ', '.join((self.user.username,
                           self.user.get_full_name(),
                           f'year={self.year_in_studies}',
                           f'track={self.track.track_number}' if self.track else 'לא הוגדר מסלול',
-                          f'took {len(self.courses.all())} courses'))
+                          f'has {len(self.courseplan_set.all())} course plans'))
 
     @property
-    def remaining(self):
-        track = self.track
-        groups = track.coursegroup_set.all()
-        required_by_type = {k: set() for k in REQUIRED_COURSE_TYPES}
-        required_courses = set()
+    def year_in_studies(self) -> Optional[int]:
+        return self.track.data_year if self.track else None
 
-        for group in groups:
-            group_courses = list(group.courses.all())
+    # @property
+    # def remaining(self):
+    #     track = self.track
+    #     groups = track.coursegroup_set.all()
+    #     required_by_type = {k: set() for k in REQUIRED_COURSE_TYPES}
+    #     required_courses = set()
 
-            required_by_type[group.course_type].update(group_courses)
-            required_courses.update(group_courses)
+    #     for group in groups:
+    #         group_courses = list(group.courses.all())
 
-        done = {t: 0 for t in ALL_COURSE_TYPES}
-        counted = set()
+    #         required_by_type[group.course_type].update(group_courses)
+    #         required_courses.update(group_courses)
 
-        for take in self.take_set.all():
-            course = take.course
-            if course not in counted:
-                counted.add(course)
-                done[take.type] += course.points
+    #     done = {t: 0 for t in ALL_COURSE_TYPES}
+    #     counted = set()
 
-        result = {CourseType.MUST.name: {'required': track.points_must,
-                                         'done': done[CourseType.MUST]},
+    #     for take in self.take_set.all():
+    #         course = take.course
+    #         if course not in counted:
+    #             counted.add(course)
+    #             done[take.type] += course.points
 
-                  CourseType.FROM_LIST.name: {'required': track.points_from_list,
-                                              'done': done[CourseType.FROM_LIST]},
+    #     result = {CourseType.MUST.name: {'required': track.points_must,
+    #                                      'done': done[CourseType.MUST]},
 
-                  CourseType.CHOICE.name: {'required': track.points_choice,
-                                           'done': done[CourseType.CHOICE]},
+    #               CourseType.FROM_LIST.name: {'required': track.points_from_list,
+    #                                           'done': done[CourseType.FROM_LIST]},
 
-                  CourseType.CORNER_STONE.name: {'required': track.points_corner_stones,
-                                                 'done': done[CourseType.CORNER_STONE]},
+    #               CourseType.CHOICE.name: {'required': track.points_choice,
+    #                                        'done': done[CourseType.CHOICE]},
 
-                  CourseType.SUPPLEMENTARY.name: {'required': track.points_complementary,
-                                                  'done': done[CourseType.SUPPLEMENTARY]}}
+    #               CourseType.CORNER_STONE.name: {'required': track.points_corner_stones,
+    #                                              'done': done[CourseType.CORNER_STONE]},
 
-        def trickle_down(trickle_from: CourseType, trickle_to: CourseType) -> None:
-            """
-            moves extra points between groups, for example, a student taking too many
-            FROM_LIST courses will have those points counted as CHOICE instead.
+    #               CourseType.SUPPLEMENTARY.name: {'required': track.points_complementary,
+    #                                               'done': done[CourseType.SUPPLEMENTARY]}}
 
-            :param trickle_from: category from which points are moved
-            :param trickle_to:category to which points are moved
-            :return: None
-            """
-            extra = result[trickle_from.name]['done'] - result[trickle_from.name]['required']
-            if extra > 0:
-                result[trickle_from.name]['done'] -= extra
-                result[trickle_to.name]['done'] += extra
+    #     def trickle_down(trickle_from: CourseType, trickle_to: CourseType) -> None:
+    #         """
+    #         moves extra points between groups, for example, a student taking too many
+    #         FROM_LIST courses will have those points counted as CHOICE instead.
 
-        trickle_down(CourseType.MUST, CourseType.CHOICE)
-        trickle_down(CourseType.FROM_LIST, CourseType.CHOICE)
-        trickle_down(CourseType.CHOICE, CourseType.SUPPLEMENTARY)
-        trickle_down(CourseType.CORNER_STONE, CourseType.SUPPLEMENTARY)
+    #         :param trickle_from: category from which points are moved
+    #         :param trickle_to:category to which points are moved
+    #         :return: None
+    #         """
+    #         extra = result[trickle_from.name]['done'] - result[trickle_from.name]['required']
+    #         if extra > 0:
+    #             result[trickle_from.name]['done'] -= extra
+    #             result[trickle_to.name]['done'] += extra
 
-        return result
+    #     trickle_down(CourseType.MUST, CourseType.CHOICE)
+    #     trickle_down(CourseType.FROM_LIST, CourseType.CHOICE)
+    #     trickle_down(CourseType.CHOICE, CourseType.SUPPLEMENTARY)
+    #     trickle_down(CourseType.CORNER_STONE, CourseType.SUPPLEMENTARY)
+
+    #     return result
+
+class CoursePlan(models.Model):
+    owner = models.ForeignKey(Student, on_delete=models.CASCADE)
+    name = models.TextField(null=True, max_length=50)
+    public = models.BooleanField(default=False)
+    created_at = models.DateTimeField(auto_now_add=True)
+    modified_at = models.DateTimeField(auto_now=True)
 
 
 class Take(models.Model):
-    student = models.ForeignKey(Student, on_delete=models.CASCADE)
+    course_plan = models.ForeignKey(CoursePlan, on_delete=models.CASCADE)
     course = models.ForeignKey(Course, on_delete=models.CASCADE)
     year_in_studies = models.IntegerField(choices=Year.choices)
     semester = models.TextField(choices=Semester.choices)

Closure_Project/rest_api/permissions.py

@@ -0,0 +1,25 @@
+from rest_framework import permissions 
+
+from rest_api.models import CoursePlan
+from rest_framework.request import HttpRequest
+
+
+def request_is_authenticated(request: HttpRequest) -> bool:
+    return bool(request.user and request.user.is_authenticated)
+
+class CoursePlanPermission(permissions.BasePermission):
+    """ Allows anyone to read the course plan if it is public.
+        The owner of the course plan can do anything with it. """
+
+    def has_permission(self, request, view):
+        if request.method in permissions.SAFE_METHODS:
+            return True
+        return request_is_authenticated(request)
+
+    def has_object_permission(self, request: HttpRequest, view, obj: CoursePlan):
+        is_authenticated = request_is_authenticated(request)
+        belongs_to_requester = is_authenticated and obj.owner.user == request.user
+        if request.method in permissions.SAFE_METHODS:
+            return belongs_to_requester or obj.public
+
+        return belongs_to_requester

Closure_Project/rest_api/serializers/CoursePlanSerializer.py

@@ -0,0 +1,39 @@
+from rest_api.serializers.DynamicSerializer import DynamicFieldsModelSerializer, serializers
+from rest_api.models import Take, CoursePlan
+
+
+class TakeSerializer(DynamicFieldsModelSerializer):
+    type = serializers.StringRelatedField(read_only=True)
+
+    class Meta:
+        model = Take
+        fields = ('course', 'course_plan_id', 'year_in_studies', 'semester', 'type')
+
+class CoursePlanSerializer(DynamicFieldsModelSerializer):
+    takes = TakeSerializer(many=True, source="take_set", partial=True)
+    owner = serializers.ReadOnlyField(source="owner.id")
+
+    class Meta:
+        model = CoursePlan
+        fields = ('id', 'owner', 'name', 'public', 'created_at', 'modified_at', 'takes')
+        read_only_fields = ('id', 'owner', 'created_at', 'modified_at')
+
+
+    def create(self, validated_data):
+        takes = validated_data.pop("take_set")
+
+        plan = CoursePlan.objects.create(**validated_data)
+        if takes:
+            for take in takes:
+                Take.objects.create(course_plan=plan, **take)
+        return plan
+    
+    def update(self, instance, validated_data):
+        takes = validated_data.pop("take_set", None)
+
+        CoursePlan.objects.filter(id=instance.id).update(**validated_data)
+        if takes:
+            Take.objects.filter(course_plan=instance).delete()
+            for take in takes:
+                Take.objects.create(course_plan=instance, **take)
+        return CoursePlan.objects.get(id=instance.id)

Closure_Project/rest_api/serializers/StudentSerializer.py

@@ -1,37 +1,36 @@
 from django.contrib.auth.models import User
 
-from rest_api.serializers.DynamicSerializer import *
+from rest_api.serializers.DynamicSerializer import serializers, DynamicFieldsModelSerializer
 from rest_api.models import Student, Take, Track
-from .TakeSerializer import TakeSerializer
+from .CoursePlanSerializer import CoursePlanSerializer
 from .TrackSerializer import TrackSerializer
 
 from django.shortcuts import get_object_or_404
 
 
 class StudentSerializer(DynamicFieldsModelSerializer):
-    courses = TakeSerializer(source='take_set', many=True)
     pk = serializers.PrimaryKeyRelatedField(source='id',
                                             read_only=True)
     username = serializers.CharField(source='user.username', read_only=True)
     track_pk = serializers.PrimaryKeyRelatedField(source='track',
                                                   queryset=Track.objects.all())
     track = TrackSerializer(fields=('track_number', 'name'), read_only=True)
-    remaining = serializers.JSONField(read_only=True)
+    course_plans = CoursePlanSerializer(source='courseplan_set', many=True, read_only=True)
 
     class Meta:
         model = Student
-        fields = ('pk', 'username', 'track_pk', 'track', 'year_in_studies', 'remaining', 'courses')
+        fields = ('pk', 'username', 'track_pk', 'track', 'course_plans', 'year_in_studies')
 
-    def update(self, student: Student, validated_data):
-        take_set = validated_data.pop('take_set')
-        student.track = validated_data.get('track', student.track)
-        student.year_in_studies = validated_data.get('year_in_studies', student.year_in_studies)
-        student.courses.clear()
-        for take in take_set:
-            Take.objects.create(student=student,
-                                course=take['course'],
-                                year_in_studies=take['year_in_studies'],
-                                semester=take['semester'])
+    # def update(self, student: Student, validated_data):
+    #     take_set = validated_data.pop('take_set')
+    #     student.track = validated_data.get('track', student.track)
+    #     student.year_in_studies = validated_data.get('year_in_studies', student.year_in_studies)
+    #     student.courses.clear()
+    #     for take in take_set:
+    #         Take.objects.create(student=student,
+    #                             course=take['course'],
+    #                             year_in_studies=take['year_in_studies'],
+    #                             semester=take['semester'])
 
-        student.save()
-        return student
+    #     student.save()
+    #     return student

Closure_Project/rest_api/serializers/TakeSerializer.py

@@ -1,13 +1,39 @@
-from rest_api.serializers.DynamicSerializer import *
-from rest_api.models import Course, Take
-from .CourseSerializer import CourseSerializer
+from rest_api.serializers.DynamicSerializer import DynamicFieldsModelSerializer, serializers
+from rest_api.models import Take, CoursePlan
 
 
 class TakeSerializer(DynamicFieldsModelSerializer):
-    course = CourseSerializer(fields=('id', 'course_id', 'name', 'semester', 'points'), read_only=True)
-    pk = serializers.PrimaryKeyRelatedField(source='course', queryset=Course.objects.all())
     type = serializers.StringRelatedField(read_only=True)
 
     class Meta:
         model = Take
-        fields = ('pk', 'course', 'year_in_studies', 'semester', 'type')
+        fields = ('course', 'course_plan_id', 'year_in_studies', 'semester', 'type')
+
+class CoursePlanSerializer(DynamicFieldsModelSerializer):
+    takes = TakeSerializer(many=True, source="take_set", partial=True)
+    owner = serializers.ReadOnlyField(source="owner.id")
+
+    class Meta:
+        model = CoursePlan
+        fields = ('id', 'owner', 'name', 'public', 'created_at', 'modified_at', 'takes')
+        read_only_fields = ('id', 'owner', 'created_at', 'modified_at')
+
+
+    def create(self, validated_data):
+        takes = validated_data.pop("take_set")
+
+        plan = CoursePlan.objects.create(**validated_data)
+        if takes:
+            for take in takes:
+                Take.objects.create(course_plan=plan, **take)
+        return plan
+    
+    def update(self, instance, validated_data):
+        takes = validated_data.pop("take_set", None)
+
+        CoursePlan.objects.filter(id=instance.id).update(**validated_data)
+        if takes:
+            Take.objects.filter(course_plan=instance).delete()
+            for take in takes:
+                Take.objects.create(course_plan=instance, **take)
+        return CoursePlan.objects.get(id=instance.id)

Closure_Project/rest_api/urls.py

@@ -17,6 +17,7 @@
 
 router.register(r'student/me', views.StudentMeViewSet, basename='Student')
 router.register(r'track_courses', views.MyTrackCourses, basename='Course')
+router.register(r'course_plans', views.CoursePlanViewSet, basename='CoursePlans')
 
 schema_view = get_schema_view(
     openapi.Info(