From bbd4416ffb00be573be0add6c3853bbdc40dbd05 Mon Sep 17 00:00:00 2001 From: Daniel Kerbel Date: Fri, 27 Aug 2021 03:59:56 +0300 Subject: [PATCH 01/49] temporary refactor that involves search-bar will most likely replace it alltogether --- .../src/components/Navigation.vue | 22 +++++++-------- .../src/components/SearchBar.vue | 28 +++++++++++++++---- .../src/components/YearSelection.vue | 4 +-- Closure_Front_End/src/views/Home.vue | 2 +- 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/Closure_Front_End/src/components/Navigation.vue b/Closure_Front_End/src/components/Navigation.vue index 8eb5e977..ba77c6ea 100644 --- a/Closure_Front_End/src/components/Navigation.vue +++ b/Closure_Front_End/src/components/Navigation.vue @@ -1,16 +1,19 @@ From a7eac2116c649db0d247bae35446a8a25560a05f Mon Sep 17 00:00:00 2001 From: Daniel Kerbel Date: Sat, 28 Aug 2021 01:47:23 +0300 Subject: [PATCH 15/49] use kebab case in FE components --- Closure_Front_End/src/components/Navigation.vue | 11 +++-------- Closure_Front_End/src/components/User.vue | 2 +- Closure_Front_End/src/views/Home.vue | 4 ++-- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/Closure_Front_End/src/components/Navigation.vue b/Closure_Front_End/src/components/Navigation.vue index 926a402f..e121c6c2 100644 --- a/Closure_Front_End/src/components/Navigation.vue +++ b/Closure_Front_End/src/components/Navigation.vue @@ -4,7 +4,7 @@
-
-
@@ -62,10 +61,6 @@ export default { const url = `tracks/${this.selectedTrack?.pk ?? 'null'}/courses/?limit=6&offset=15&data_year=${this.selectedYear}&search=${query}`; return await fetchDjangoListIntoSelectOptions(this.http, url, course => course.name); }, - selectCourseHandler(course) { - // this.$refs.courseMultiselect.clear(); - this.selectCourse(course); - } } }; diff --git a/Closure_Front_End/src/components/User.vue b/Closure_Front_End/src/components/User.vue index c228920e..3f777f04 100644 --- a/Closure_Front_End/src/components/User.vue +++ b/Closure_Front_End/src/components/User.vue @@ -31,7 +31,7 @@
  • - - - + +
    From 156811b611bb7a4e547a9446f77c9bab99e68352 Mon Sep 17 00:00:00 2001 From: Daniel Kerbel Date: Sat, 28 Aug 2021 22:23:48 +0300 Subject: [PATCH 16/49] 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? --- Closure_Project/rest_api/admin.py | 6 +- .../migrations/0005_auto_20210828_2210.py | 43 ++++ Closure_Project/rest_api/models.py | 106 ++++---- Closure_Project/rest_api/permissions.py | 25 ++ .../serializers/CoursePlanSerializer.py | 39 +++ .../rest_api/serializers/StudentSerializer.py | 33 ++- .../rest_api/serializers/TakeSerializer.py | 38 ++- Closure_Project/rest_api/urls.py | 1 + Closure_Project/rest_api/views.py | 50 +++- Closure_Project/tests/test_courseplan.py | 229 ++++++++++++++++++ Closure_Project/tests/test_models.py | 10 +- 11 files changed, 495 insertions(+), 85 deletions(-) create mode 100644 Closure_Project/rest_api/migrations/0005_auto_20210828_2210.py create mode 100644 Closure_Project/rest_api/permissions.py create mode 100644 Closure_Project/rest_api/serializers/CoursePlanSerializer.py create mode 100644 Closure_Project/tests/test_courseplan.py diff --git a/Closure_Project/rest_api/admin.py b/Closure_Project/rest_api/admin.py index b15bc690..4e9087d6 100644 --- a/Closure_Project/rest_api/admin.py +++ b/Closure_Project/rest_api/admin.py @@ -1,12 +1,10 @@ from django.contrib import admin # 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) \ No newline at end of file diff --git a/Closure_Project/rest_api/migrations/0005_auto_20210828_2210.py b/Closure_Project/rest_api/migrations/0005_auto_20210828_2210.py new file mode 100644 index 00000000..29acb6d0 --- /dev/null +++ b/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, + ), + ] diff --git a/Closure_Project/rest_api/models.py b/Closure_Project/rest_api/models.py index bb984350..0214c04d 100644 --- a/Closure_Project/rest_api/models.py +++ b/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) diff --git a/Closure_Project/rest_api/permissions.py b/Closure_Project/rest_api/permissions.py new file mode 100644 index 00000000..a1220a56 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/Closure_Project/rest_api/serializers/CoursePlanSerializer.py b/Closure_Project/rest_api/serializers/CoursePlanSerializer.py new file mode 100644 index 00000000..949728c9 --- /dev/null +++ b/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) \ No newline at end of file diff --git a/Closure_Project/rest_api/serializers/StudentSerializer.py b/Closure_Project/rest_api/serializers/StudentSerializer.py index e86dedda..ae1a563d 100644 --- a/Closure_Project/rest_api/serializers/StudentSerializer.py +++ b/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 diff --git a/Closure_Project/rest_api/serializers/TakeSerializer.py b/Closure_Project/rest_api/serializers/TakeSerializer.py index 7bfe58cc..949728c9 100644 --- a/Closure_Project/rest_api/serializers/TakeSerializer.py +++ b/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) \ No newline at end of file diff --git a/Closure_Project/rest_api/urls.py b/Closure_Project/rest_api/urls.py index 0ac64115..e92874f7 100644 --- a/Closure_Project/rest_api/urls.py +++ b/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( diff --git a/Closure_Project/rest_api/views.py b/Closure_Project/rest_api/views.py index a2259950..182f5cb6 100644 --- a/Closure_Project/rest_api/views.py +++ b/Closure_Project/rest_api/views.py @@ -1,22 +1,25 @@ # Create your views here -from django.db.models import Case, When +from django.db.models import Case, When, Q +from django.utils.timezone import make_aware from rest_framework import filters from rest_framework import viewsets from rest_framework.decorators import action from rest_framework.permissions import IsAuthenticated, IsAdminUser from django_filters.rest_framework import DjangoFilterBackend +from rest_framework.request import HttpRequest from rest_framework.response import Response - +from datetime import datetime from rest_framework_nested.viewsets import NestedViewSetMixin -from .models import Course, CourseType, Student, CourseGroup, Track, Take +from .permissions import CoursePlanPermission +from .models import Course, CourseType, Student, CourseGroup, Track, Take, CoursePlan from .pagination import ResultSetPagination from .serializers.CourseSerializer import CourseSerializer from .serializers.CourseGroupSerializer import CourseGroupSerializer from .serializers.StudentSerializer import StudentSerializer from .serializers.TrackSerializer import TrackSerializer, TrackSerializerWithCourseGroups -from .serializers.TakeSerializer import TakeSerializer +from .serializers.CoursePlanSerializer import TakeSerializer, CoursePlanSerializer class CourseViewSet(viewsets.ModelViewSet): @@ -107,11 +110,15 @@ class StudentGroupViewSet(viewsets.ModelViewSet): queryset = Student.objects.all().order_by('user__username') serializer_class = StudentSerializer filter_backends = (filters.SearchFilter, DjangoFilterBackend) - filter_fields = ('user__username', 'year_in_studies', 'track__track_number', - 'courses__course_id') + # filter_fields = ('user__username', 'year_in_studies', 'track__track_number', + filter_fields = ('user__username', 'track__track_number', + # 'courses__course_id') + ) pagination_class = ResultSetPagination - search_fields = ('user__username', 'year_in_studies', '^track__track_number', - '^courses__course_id') + # search_fields = ('user__username', 'year_in_studies', '^track__track_number', + search_fields = ('user__username', '^track__track_number', + # '^courses__course_id') + ) class TrackViewSet(viewsets.ModelViewSet): @@ -129,6 +136,31 @@ def get_serializer_class(self): return TrackSerializerWithCourseGroups class TakeGroupViewSet(viewsets.ModelViewSet): - permission_classes = (IsAuthenticated,) + permission_classes = (IsAdminUser,) queryset = Take.objects.all().order_by('course') serializer_class = TakeSerializer + +class CoursePlanViewSet(viewsets.ModelViewSet): + permission_classes = (CoursePlanPermission,) + serializer_class = CoursePlanSerializer + + def get_queryset(self): + + public_plans = Q(public=True) + + if not self.request.user or not self.request.user.is_authenticated: + return CoursePlan.objects.filter(public_plans) + student = Student.objects.get(user=self.request.user.id) + student = get_request_student(self.request) + return CoursePlan.objects.filter(Q(owner=student) | public_plans) + + def perform_create(self, serializer): + student = get_request_student(self.request) + serializer.save(owner=student) + + def perform_update(self, serializer): + serializer.save(modified_at=make_aware(datetime.now())) + + +def get_request_student(req: HttpRequest) -> Student: + return Student.objects.get(user=req.user.id) \ No newline at end of file diff --git a/Closure_Project/tests/test_courseplan.py b/Closure_Project/tests/test_courseplan.py new file mode 100644 index 00000000..87bbaf2f --- /dev/null +++ b/Closure_Project/tests/test_courseplan.py @@ -0,0 +1,229 @@ +from typing import Any, Dict +import pytest +import json +import io +from rest_framework.test import APIRequestFactory, force_authenticate +from rest_framework import status + +from rest_api import models +from rest_api.views import CoursePlanViewSet +from rest_api.serializers.CoursePlanSerializer import CoursePlanSerializer + +def create_dummy_student(username: str) -> models.Student: + user = models.User(username=username) + user.save() + owner = models.Student.objects.get(user=user) + return owner + +@pytest.fixture +def courses(): + courses = [ + models.Course(course_id=1330, data_year=2015, name="Course A", semester=models.Semester.A, is_given_this_year=True, points=4), + models.Course(course_id=1331, data_year=2015, name="Course B", semester=models.Semester.B, is_given_this_year=True, points=4), + models.Course(course_id=1332, data_year=2015, name="Course C", semester=models.Semester.A, is_given_this_year=True, points=4), + models.Course(course_id=1333, data_year=2015, name="Course D", semester=models.Semester.EITHER, is_given_this_year=True, points=4), + ] + models.Course.objects.bulk_create(courses) + return models.Course.objects.all() + + + +@pytest.mark.django_db +def test_courseplan_serializer(courses): + owner = create_dummy_student("Stu A") + # print("courses", courses) + # print("courseplan serializer", repr(CoursePlanSerializer())) + + # deserialization + course_plan = { + "public": False, + "takes": [ + { "course": courses[0].id, "semester": models.Semester.A.value, "year_in_studies": models.Year.FIRST.value }, + { "course": courses[1].id, "semester": models.Semester.B.value, "year_in_studies": models.Year.FIRST.value }, + { "course": courses[3].id, "semester": models.Semester.A.value, "year_in_studies": models.Year.SECOND.value } + ] + } + + deserializer = CoursePlanSerializer(data=course_plan) + assert deserializer.is_valid(), f"Gotten errors: {deserializer.errors}" + # print("deserializer validated_data", deserializer.validated_data) + model_course_plan = deserializer.save(owner=owner) + model_takes = model_course_plan.take_set.all() + + assert len(model_takes) == len(course_plan["takes"]), "The deserialized model object should match the data object" + assert all(take.course_plan_id == model_course_plan.id + for take in model_takes), "The deserialized model object should match the data object" + + assert model_course_plan.id is not None, "Deserialized model object should have an ID" + assert model_course_plan.owner.id == owner.id + + # print("model_takes", model_takes) + + # very simple testing of serialization + serializer = CoursePlanSerializer(model_course_plan) + serialization_result = serializer.data + + assert len(serialization_result["takes"]) == len(course_plan["takes"]) + + # now, updating the previous instance, adding a title and changing the takes + course_plan_2 = { + "id": model_course_plan.id, + "name": "now i have a name", + "public": False, + "takes": [ + { "course": courses[0].id, "semester": models.Semester.A.value, "year_in_studies": models.Year.FIRST.value }, + { "course": courses[2].id, "semester": models.Semester.A.value, "year_in_studies": models.Year.SECOND.value }, + ] + } + + + deserializer_2 = CoursePlanSerializer(model_course_plan, data=course_plan_2) + assert deserializer_2.is_valid() + model_course_plan_2 = deserializer_2.save() + + assert models.CoursePlan.objects.count() == 1 + assert models.Take.objects.count() == 2, "Old takes should have been deleted" + assert model_course_plan_2.name == course_plan_2["name"], "should have a name now" + + +@pytest.fixture +def view(): + return CoursePlanViewSet.as_view(actions={'post': 'create', 'put': 'update', 'get': 'retrieve', 'patch': 'partial_update'}) + +@pytest.fixture +def stud1(): + return create_dummy_student("alice") + +@pytest.fixture +def stud2(): + return create_dummy_student("bob") + +@pytest.fixture +def stud1_plan(courses): + return { + "name": "a private plan", + "public": False, + "takes": [ + { "course": courses[0].id, "semester": models.Semester.A.value, "year_in_studies": models.Year.FIRST.value }, + { "course": courses[1].id, "semester": models.Semester.B.value, "year_in_studies": models.Year.FIRST.value }, + { "course": courses[3].id, "semester": models.Semester.A.value, "year_in_studies": models.Year.SECOND.value } + ] + } + +@pytest.fixture +def stud2_plan(courses): + return { + "name": "a public plan", + "public": True, + "takes": [ + { "course": courses[0].id, "semester": models.Semester.A.value, "year_in_studies": models.Year.FIRST.value }, + { "course": courses[2].id, "semester": models.Semester.A.value, "year_in_studies": models.Year.SECOND.value }, + ] + } + +@pytest.fixture +def factory(): + return APIRequestFactory() + + +@pytest.fixture +def post_plan(factory, view): + def _fun(plan, student: models.Student) -> Dict[Any, Any]: + req = factory.post('/course_plans/', plan, format='json') + force_authenticate(req, student.user) + res = view(req) + assert res.status_code == status.HTTP_201_CREATED, "Creating a course plan should succeed" + + res.render() + post_result = json.load(io.BytesIO(res.content)) + assert post_result["owner"] == student.id, "The owner field should be automatically populated" + assert post_result["owner"] == student.id, "The owner field should be automatically populated" + return post_result + + return _fun + +@pytest.mark.django_db +def test_courseplan_post(post_plan, stud1, stud1_plan): + post_plan(stud1_plan, stud1) + +@pytest.mark.django_db +def test_courseplan_partial_update(view, stud1, stud1_plan, post_plan): + factory = APIRequestFactory() + + # post + post_result = post_plan(stud1_plan, stud1) + + # partial update + stud1_req2 = factory.patch('/course_plans/', { "name": "cool name"}, format='json') + force_authenticate(stud1_req2, stud1.user) + res2 = view(stud1_req2, pk = post_result["id"]) + + assert res2.status_code == status.HTTP_200_OK, "Patching a course plan should succeed" + res2.render() + patch_result = json.load(io.BytesIO(res2.content)) + + print("patch_result", patch_result) + assert patch_result["name"] == "cool name", "patched name was changed" + assert patch_result["modified_at"] > patch_result["created_at"] + assert len(patch_result["takes"]) == len(stud1_plan["takes"]), "a patch that doesn't include takes, does not change them" + + + + + + +@pytest.mark.django_db +def test_courseplan_viewset_permissions(courses, view, stud1, stud2, stud1_plan, stud2_plan, post_plan): + factory = APIRequestFactory() + + # setup, create a plan for each student + plan1_res = post_plan(stud1_plan, stud1) + plan2_res = post_plan(stud2_plan, stud2) + print("res1", plan1_res) + print("res2", plan2_res) + + # stud1 can access his own plan + stud1_req2 = factory.get('/course_plans/') + force_authenticate(stud1_req2, stud1.user) + res3 = view(stud1_req2, pk=plan1_res["id"]) + assert res3.status_code == status.HTTP_200_OK + + res3.render() + plan1_res2 = json.load(io.BytesIO(res3.content)) + assert plan1_res2 == plan1_res, "The retrieved plan is identical to the one created" + + # and also access stud2's plan, which is public + stud1_req3 = factory.get('/course_plans/') + force_authenticate(stud1_req3, stud1.user) + res4 = view(stud1_req3, pk=plan2_res["id"]) + assert res4.status_code == status.HTTP_200_OK, "Student 1 can access a public plan" + res4.render() + res4_data = json.load(io.BytesIO(res4.content)) + assert res4_data == plan2_res, "The retrieved plan is identical to the one created" + + # which can also be accessed without being authenticated at all + unauth_req = factory.get('/course_plans/') + res5 = view(unauth_req, pk=plan2_res["id"]) + assert res5.status_code == status.HTTP_200_OK, "An unauthenticated user can access a public plan" + res5.render() + res5_data = json.load(io.BytesIO(res5.content)) + assert res5_data == plan2_res, "The retrieved plan is identical to the one created" + + # however, stud1 cannot modify stud2's plan + stud1_modify_req = factory.patch("/course_plans/") + force_authenticate(stud1_modify_req, stud1.user) + modify_res = view(stud1_modify_req, pk=plan2_res["id"]) + assert modify_res.status_code == status.HTTP_403_FORBIDDEN, "A plan can only be modified by its user" + + # however, stud2 cannot access stud1's plan + stud2_req2 = factory.get('/course_plans/') + force_authenticate(stud2_req2, stud2.user) + res6 = view(stud2_req2, pk=plan1_res["id"]) + assert res6.status_code == status.HTTP_404_NOT_FOUND, \ + "An authenticated user cannot access another student's private plan" + + # nor can unauthenticated users access stud1's plan + unauth_req2 = factory.get('/course_plans/') + res7 = view(unauth_req2, pk=plan1_res["id"]) + assert res7.status_code == status.HTTP_404_NOT_FOUND, \ + "An unauthenticated user cannot access another student's private plan" diff --git a/Closure_Project/tests/test_models.py b/Closure_Project/tests/test_models.py index fb5cff79..8fac046a 100644 --- a/Closure_Project/tests/test_models.py +++ b/Closure_Project/tests/test_models.py @@ -12,4 +12,12 @@ def test_can_create_course(): course.save() course2 = models.Course.objects.get(course_id=13371337) - assert course2.name == "Software Testing" \ No newline at end of file + assert course2.name == "Software Testing" + +@pytest.mark.django_db +def test_user_creation_creates_student(): + user = models.User(username="Dummy") + user.save() + owner = models.Student.objects.filter(user=user) + assert len(owner) == 1 + print(owner) From c7c4b34d1b58cd38d8b35e2011b7e566fb139742 Mon Sep 17 00:00:00 2001 From: Daniel Kerbel Date: Sat, 28 Aug 2021 22:54:05 +0300 Subject: [PATCH 17/49] Forgot to remove TakeSerializer (renamed to CoursePlanSerializer) --- .../rest_api/serializers/TakeSerializer.py | 39 ------------------- 1 file changed, 39 deletions(-) delete mode 100644 Closure_Project/rest_api/serializers/TakeSerializer.py diff --git a/Closure_Project/rest_api/serializers/TakeSerializer.py b/Closure_Project/rest_api/serializers/TakeSerializer.py deleted file mode 100644 index 949728c9..00000000 --- a/Closure_Project/rest_api/serializers/TakeSerializer.py +++ /dev/null @@ -1,39 +0,0 @@ -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) \ No newline at end of file From 7c793d21354937a98762a0415b829717ff5cffae Mon Sep 17 00:00:00 2001 From: Daniel Kerbel Date: Sat, 28 Aug 2021 23:09:45 +0300 Subject: [PATCH 18/49] move 'track' and 'remaining' from Student into CoursePlan --- ...828_2210.py => 0005_auto_20210828_2323.py} | 8 +- Closure_Project/rest_api/models.py | 135 +++++++++--------- .../serializers/CoursePlanSerializer.py | 6 +- .../rest_api/serializers/StudentSerializer.py | 21 +-- Closure_Project/rest_api/views.py | 10 +- 5 files changed, 81 insertions(+), 99 deletions(-) rename Closure_Project/rest_api/migrations/{0005_auto_20210828_2210.py => 0005_auto_20210828_2323.py} (78%) diff --git a/Closure_Project/rest_api/migrations/0005_auto_20210828_2210.py b/Closure_Project/rest_api/migrations/0005_auto_20210828_2323.py similarity index 78% rename from Closure_Project/rest_api/migrations/0005_auto_20210828_2210.py rename to Closure_Project/rest_api/migrations/0005_auto_20210828_2323.py index 29acb6d0..3a638d06 100644 --- a/Closure_Project/rest_api/migrations/0005_auto_20210828_2210.py +++ b/Closure_Project/rest_api/migrations/0005_auto_20210828_2323.py @@ -1,4 +1,4 @@ -# Generated by Django 3.2.4 on 2021-08-28 19:10 +# Generated by Django 3.2.4 on 2021-08-28 20:23 from django.db import migrations, models import django.db.models.deletion @@ -15,6 +15,10 @@ class Migration(migrations.Migration): model_name='student', name='courses', ), + migrations.RemoveField( + model_name='student', + name='track', + ), migrations.RemoveField( model_name='student', name='year_in_studies', @@ -31,7 +35,9 @@ class Migration(migrations.Migration): ('public', models.BooleanField(default=False)), ('created_at', models.DateTimeField(auto_now_add=True)), ('modified_at', models.DateTimeField(auto_now=True)), + ('courses', models.ManyToManyField(blank=True, through='rest_api.Take', to='rest_api.Course')), ('owner', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='rest_api.student')), + ('track', models.ForeignKey(null=True, on_delete=django.db.models.deletion.CASCADE, to='rest_api.track')), ], ), migrations.AddField( diff --git a/Closure_Project/rest_api/models.py b/Closure_Project/rest_api/models.py index 0214c04d..b318874b 100644 --- a/Closure_Project/rest_api/models.py +++ b/Closure_Project/rest_api/models.py @@ -1,7 +1,6 @@ # Create your models here. -import uuid from enum import Enum -from typing import List, Optional +from typing import Any, List, Dict, Optional from django.core.exceptions import ValidationError @@ -204,85 +203,85 @@ 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) - # 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'has {len(self.courseplan_set.all())} course plans')) - @property - def year_in_studies(self) -> Optional[int]: - return self.track.data_year if self.track else None - - # @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() - - # for group in groups: - # group_courses = list(group.courses.all()) - - # required_by_type[group.course_type].update(group_courses) - # required_courses.update(group_courses) - - # done = {t: 0 for t in ALL_COURSE_TYPES} - # counted = set() - - # for take in self.take_set.all(): - # course = take.course - # if course not in counted: - # counted.add(course) - # done[take.type] += course.points - - # result = {CourseType.MUST.name: {'required': track.points_must, - # 'done': done[CourseType.MUST]}, - - # CourseType.FROM_LIST.name: {'required': track.points_from_list, - # 'done': done[CourseType.FROM_LIST]}, - - # CourseType.CHOICE.name: {'required': track.points_choice, - # 'done': done[CourseType.CHOICE]}, - - # CourseType.CORNER_STONE.name: {'required': track.points_corner_stones, - # 'done': done[CourseType.CORNER_STONE]}, - - # CourseType.SUPPLEMENTARY.name: {'required': track.points_complementary, - # 'done': done[CourseType.SUPPLEMENTARY]}} - - # 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. - - # :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 - - # 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) + track = models.ForeignKey(Track, on_delete=models.CASCADE, null=True) + courses = models.ManyToManyField(Course, through='Take', blank=True) + + @property + def year_in_studies(self) -> Optional[int]: + return self.track.data_year if self.track else None + @property + def remaining(self) -> Optional[Dict[str, Any]]: + track = self.track + if not track: + return None + groups = track.coursegroup_set.all() + required_by_type = {k: set() for k in REQUIRED_COURSE_TYPES} + required_courses = set() + + for group in groups: + group_courses = list(group.courses.all()) + + required_by_type[group.course_type].update(group_courses) + required_courses.update(group_courses) + + done = {t: 0 for t in ALL_COURSE_TYPES} + counted = set() + + for take in self.take_set.all(): + course = take.course + if course not in counted: + counted.add(course) + done[take.type] += course.points + + result = {CourseType.MUST.name: {'required': track.points_must, + 'done': done[CourseType.MUST]}, + + CourseType.FROM_LIST.name: {'required': track.points_from_list, + 'done': done[CourseType.FROM_LIST]}, + + CourseType.CHOICE.name: {'required': track.points_choice, + 'done': done[CourseType.CHOICE]}, + + CourseType.CORNER_STONE.name: {'required': track.points_corner_stones, + 'done': done[CourseType.CORNER_STONE]}, + + CourseType.SUPPLEMENTARY.name: {'required': track.points_complementary, + 'done': done[CourseType.SUPPLEMENTARY]}} + + 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. + + :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 + + 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 Take(models.Model): course_plan = models.ForeignKey(CoursePlan, on_delete=models.CASCADE) diff --git a/Closure_Project/rest_api/serializers/CoursePlanSerializer.py b/Closure_Project/rest_api/serializers/CoursePlanSerializer.py index 949728c9..63fc2e0d 100644 --- a/Closure_Project/rest_api/serializers/CoursePlanSerializer.py +++ b/Closure_Project/rest_api/serializers/CoursePlanSerializer.py @@ -13,10 +13,12 @@ class CoursePlanSerializer(DynamicFieldsModelSerializer): takes = TakeSerializer(many=True, source="take_set", partial=True) owner = serializers.ReadOnlyField(source="owner.id") + remaining = serializers.JSONField(read_only=True) + class Meta: model = CoursePlan - fields = ('id', 'owner', 'name', 'public', 'created_at', 'modified_at', 'takes') - read_only_fields = ('id', 'owner', 'created_at', 'modified_at') + fields = ('id', 'owner', 'name', 'track', 'public', 'remaining', 'created_at', 'modified_at', 'takes') + read_only_fields = ('id', 'created_at', 'modified_at') def create(self, validated_data): diff --git a/Closure_Project/rest_api/serializers/StudentSerializer.py b/Closure_Project/rest_api/serializers/StudentSerializer.py index ae1a563d..ca37f102 100644 --- a/Closure_Project/rest_api/serializers/StudentSerializer.py +++ b/Closure_Project/rest_api/serializers/StudentSerializer.py @@ -9,28 +9,9 @@ class StudentSerializer(DynamicFieldsModelSerializer): - 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) course_plans = CoursePlanSerializer(source='courseplan_set', many=True, read_only=True) class Meta: model = Student - 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']) - - # student.save() - # return student + fields = ('id', 'username', 'course_plans') diff --git a/Closure_Project/rest_api/views.py b/Closure_Project/rest_api/views.py index 182f5cb6..2a0c5ae7 100644 --- a/Closure_Project/rest_api/views.py +++ b/Closure_Project/rest_api/views.py @@ -110,15 +110,9 @@ class StudentGroupViewSet(viewsets.ModelViewSet): queryset = Student.objects.all().order_by('user__username') serializer_class = StudentSerializer filter_backends = (filters.SearchFilter, DjangoFilterBackend) - # filter_fields = ('user__username', 'year_in_studies', 'track__track_number', - filter_fields = ('user__username', 'track__track_number', - # 'courses__course_id') - ) + filter_fields = ('user__username',) pagination_class = ResultSetPagination - # search_fields = ('user__username', 'year_in_studies', '^track__track_number', - search_fields = ('user__username', '^track__track_number', - # '^courses__course_id') - ) + search_fields = ('user__username',) class TrackViewSet(viewsets.ModelViewSet): From 656bebe2f71f0b2ba69fcb399e4ddd5ee3b329de Mon Sep 17 00:00:00 2001 From: Daniel Kerbel Date: Sun, 29 Aug 2021 20:56:00 +0300 Subject: [PATCH 19/49] Add "IsAdminOrAuthenticatedReadOnly" permission This ensures ordinary users cannot modify tracks/courses, but can still fetch them --- Closure_Project/rest_api/permissions.py | 11 ++++++++++- Closure_Project/rest_api/views.py | 6 +++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/Closure_Project/rest_api/permissions.py b/Closure_Project/rest_api/permissions.py index a1220a56..45742a56 100644 --- a/Closure_Project/rest_api/permissions.py +++ b/Closure_Project/rest_api/permissions.py @@ -22,4 +22,13 @@ def has_object_permission(self, request: HttpRequest, view, obj: CoursePlan): if request.method in permissions.SAFE_METHODS: return belongs_to_requester or obj.public - return belongs_to_requester \ No newline at end of file + return belongs_to_requester + +class IsAdminOrAuthenticatedReadOnly(permissions.IsAdminUser): + """ Allows read-only access to authenticated users, and update access + to admins only. """ + def has_permission(self, request, view): + if request.method in permissions.SAFE_METHODS: + return request_is_authenticated(request) + + return super().has_permission(request, view) \ No newline at end of file diff --git a/Closure_Project/rest_api/views.py b/Closure_Project/rest_api/views.py index 2a0c5ae7..712ff813 100644 --- a/Closure_Project/rest_api/views.py +++ b/Closure_Project/rest_api/views.py @@ -11,7 +11,7 @@ from datetime import datetime from rest_framework_nested.viewsets import NestedViewSetMixin -from .permissions import CoursePlanPermission +from .permissions import CoursePlanPermission, IsAdminOrAuthenticatedReadOnly from .models import Course, CourseType, Student, CourseGroup, Track, Take, CoursePlan from .pagination import ResultSetPagination @@ -23,7 +23,7 @@ class CourseViewSet(viewsets.ModelViewSet): - permission_classes = (IsAuthenticated,) + permission_classes = (IsAdminOrAuthenticatedReadOnly,) queryset = Course.objects.all().order_by('course_id') serializer_class = CourseSerializer filter_backends = (DjangoFilterBackend, filters.SearchFilter) @@ -116,7 +116,7 @@ class StudentGroupViewSet(viewsets.ModelViewSet): class TrackViewSet(viewsets.ModelViewSet): - permission_classes = (IsAuthenticated,) + permission_classes = (IsAdminOrAuthenticatedReadOnly,) queryset = Track.objects.all().order_by('track_number') serializer_class = TrackSerializer filter_backends = (DjangoFilterBackend, filters.SearchFilter) From 9456810a4abb9730bfa8ea92107e7a57daacdb95 Mon Sep 17 00:00:00 2001 From: Daniel Kerbel Date: Sun, 29 Aug 2021 20:57:56 +0300 Subject: [PATCH 20/49] Show course type for courses by tracks endpoint only type is no longer dependant on the authenticated student --- .../rest_api/serializers/CourseSerializer.py | 23 +++++++++++-------- Closure_Project/rest_api/views.py | 3 ++- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/Closure_Project/rest_api/serializers/CourseSerializer.py b/Closure_Project/rest_api/serializers/CourseSerializer.py index c9dc2db5..412d06ba 100644 --- a/Closure_Project/rest_api/serializers/CourseSerializer.py +++ b/Closure_Project/rest_api/serializers/CourseSerializer.py @@ -1,22 +1,27 @@ from rest_api.serializers.DynamicSerializer import DynamicFieldsModelSerializer from rest_framework import serializers -from rest_api.models import Course, Student +from rest_api.models import Course, Track from rest_api.rest_utils import get_course_type class CourseSerializer(DynamicFieldsModelSerializer): pk = serializers.PrimaryKeyRelatedField(source='id', read_only=True) + + class Meta: + model = Course + fields = ('pk', 'course_id', 'data_year', 'name', 'semester', 'points', 'is_given_this_year', + 'is_corner_stone', 'comment') + +class CourseOfTrackSerializer(CourseSerializer): type = serializers.SerializerMethodField(method_name='get_type') def get_type(self, obj): - user = self.context.get('request').user - try: - student = Student.objects.get(user=user) - except Student.DoesNotExist: - return None - return get_course_type(student.track, obj) + request = self.context['request'] + track_id = request.parser_context["kwargs"]["track_pk"] + track = Track.objects.get(id=track_id) + return get_course_type(track, obj) + class Meta: model = Course - fields = ('pk', 'course_id', 'data_year', 'name', 'semester', 'points', 'type', 'is_given_this_year', - 'is_corner_stone', 'comment') + fields = CourseSerializer.Meta.fields + ('type', ) \ No newline at end of file diff --git a/Closure_Project/rest_api/views.py b/Closure_Project/rest_api/views.py index 712ff813..1eb98fa6 100644 --- a/Closure_Project/rest_api/views.py +++ b/Closure_Project/rest_api/views.py @@ -15,7 +15,7 @@ from .models import Course, CourseType, Student, CourseGroup, Track, Take, CoursePlan from .pagination import ResultSetPagination -from .serializers.CourseSerializer import CourseSerializer +from .serializers.CourseSerializer import CourseSerializer, CourseOfTrackSerializer from .serializers.CourseGroupSerializer import CourseGroupSerializer from .serializers.StudentSerializer import StudentSerializer from .serializers.TrackSerializer import TrackSerializer, TrackSerializerWithCourseGroups @@ -33,6 +33,7 @@ class CourseViewSet(viewsets.ModelViewSet): class TrackCoursesViewSet(CourseViewSet, NestedViewSetMixin): filter_fields = CourseViewSet.filter_fields + ('coursegroup__course_type', ) + serializer_class = CourseOfTrackSerializer def get_queryset(self): if getattr(self, 'swagger_fake_view', False): relevant_cgs = CourseGroup.objects.all() From ec421871681dbcea0c110a7ef92892b27681f808 Mon Sep 17 00:00:00 2001 From: Daniel Kerbel Date: Sun, 29 Aug 2021 20:58:11 +0300 Subject: [PATCH 21/49] Take type depends on course_plan track, not student --- Closure_Project/rest_api/models.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Closure_Project/rest_api/models.py b/Closure_Project/rest_api/models.py index b318874b..e5f49aa0 100644 --- a/Closure_Project/rest_api/models.py +++ b/Closure_Project/rest_api/models.py @@ -295,6 +295,8 @@ def __str__(self): f'semester={self.semester.lower()}')) @property - def type(self) -> CourseType: + def type(self) -> Optional[CourseType]: from rest_api.rest_utils import get_course_type - return get_course_type(self.student.track, self.course) + if self.course_plan.track: + return get_course_type(self.course_plan.track, self.course) + return None From 67b044b83beb747d371b609224987c0eea0ccaf0 Mon Sep 17 00:00:00 2001 From: Daniel Kerbel Date: Tue, 31 Aug 2021 00:59:10 +0300 Subject: [PATCH 22/49] standardize take of a course in FE - semester is now stored as a string(same enum as in BE), and not ints. This makes some utility functions redundant and simplifies processing. - The 'semester' and 'year' fields of Course, have been moved to the 'take' object field of a Course. Now, the Course 'data_year' and 'semester' fields purely reflect the course's state in the DB. This distinction is important for courses with any "any" field, whose assigned take in pratice(sem1/sem2) may be different from the one stated in the DB, as well as annual semesters which are currently classified as year 1. --- Closure_Front_End/src/components/Semester.vue | 2 +- Closure_Front_End/src/components/Year.vue | 4 ++-- Closure_Front_End/src/course-store.js | 4 ++-- .../src/huji-import/CandidateCourses.vue | 6 +++--- .../src/huji-import/ParsedCoursesTables.vue | 14 +++++++++----- Closure_Front_End/src/utils.js | 15 --------------- Closure_Front_End/src/views/CourseImport.vue | 10 ++++++---- 7 files changed, 23 insertions(+), 32 deletions(-) diff --git a/Closure_Front_End/src/components/Semester.vue b/Closure_Front_End/src/components/Semester.vue index 6a720f58..e458eb00 100644 --- a/Closure_Front_End/src/components/Semester.vue +++ b/Closure_Front_End/src/components/Semester.vue @@ -50,7 +50,7 @@ export default { return this.courses .filter( (course) => - course.year == this.year.id && course.semester == this.semester.id + course.take.year === this.year.id && course.take.semester === this.semester.id ) .sort((c1, c2) => c1.type - c2.type); }, diff --git a/Closure_Front_End/src/components/Year.vue b/Closure_Front_End/src/components/Year.vue index fed606eb..ec9661d5 100644 --- a/Closure_Front_End/src/components/Year.vue +++ b/Closure_Front_End/src/components/Year.vue @@ -26,8 +26,8 @@ export default { data() { return { semesters: [ - { id: 1, name: "סמסטר א" }, - { id: 2, name: "סמסטר ב" }, + { id: "FIRST", name: "סמסטר א" }, + { id: "SECOND", name: "סמסטר ב" }, ] }; }, diff --git a/Closure_Front_End/src/course-store.js b/Closure_Front_End/src/course-store.js index 00c144a6..3fcc92fc 100644 --- a/Closure_Front_End/src/course-store.js +++ b/Closure_Front_End/src/course-store.js @@ -28,8 +28,8 @@ export const moveCourse = ({course, newYear, newSemester}) => { if (!course) { throw new Error("Course must not be undefined/null") } - course.year = newYear - course.semester = newSemester + course.take.year = newYear + course.take.semester = newSemester } export const deleteCourse = (course) => { diff --git a/Closure_Front_End/src/huji-import/CandidateCourses.vue b/Closure_Front_End/src/huji-import/CandidateCourses.vue index 0d5f6c72..c79e448b 100644 --- a/Closure_Front_End/src/huji-import/CandidateCourses.vue +++ b/Closure_Front_End/src/huji-import/CandidateCourses.vue @@ -46,11 +46,11 @@
    @@ -110,7 +110,7 @@ export default { /** The semester must be set for all courses in order to begin the import */ canImport() { - return this.courses.every(course => course.semester) + return this.courses.every(course => course.take.semester) }, /** If the user has already inserted courses before beginning import, diff --git a/Closure_Front_End/src/huji-import/ParsedCoursesTables.vue b/Closure_Front_End/src/huji-import/ParsedCoursesTables.vue index 14d73795..433509c9 100644 --- a/Closure_Front_End/src/huji-import/ParsedCoursesTables.vue +++ b/Closure_Front_End/src/huji-import/ParsedCoursesTables.vue @@ -30,19 +30,22 @@ const processCourses = async (parsedCourses, http) => { console.error(res.data.results) } const gottenCourse = res.data.results[0] - const finalCourse = { ... course, name: gottenCourse.name, type: gottenCourse.type } + const finalCourse = { ...gottenCourse, + take: { year: course.year, semester: course.semester }, + statistics_url: course.statistics_url + }; if (["FIRST", "SECOND"].includes(gottenCourse.semester)) { if (course.semester && course.semester !== gottenCourse.semester) { console.warn(`Course ${course.course_id} - ${course.name} at year ${course.year} is offered only in semester ` +`${gottenCourse.semester}, but student took it in ${course.semester}`) - finalCourse.semester = course.semester + finalCourse.take.semester = course.semester } else { - finalCourse.semester = gottenCourse.semester + finalCourse.take.semester = gottenCourse.semester } } else if (gottenCourse.semester === "EITHER") { finalCourse.ambiguous = !course.semester } else if (["ANNUAL", "SUMMER"].includes(gottenCourse.semester)) { - finalCourse.semester = 'FIRST' + finalCourse.take.semester = 'FIRST' console.warn(`Course ${course.course_id} - ${course.name} at year ${course.year} which is offered at unsupported period ` +`${gottenCourse.semester}, will be considered as part of the first semester`) } @@ -65,8 +68,9 @@ export default { const finishedProcessing = ref(false) onMounted(async () => { - console.log(`processing ${parsedCourses.value.length} courses`); + console.log(`processing ${parsedCourses.value.length} courses scraped from HUJI site:`, parsedCourses.value); const processedCourses = await processCourses(parsedCourses, http) + console.log(`processed courses`, processedCourses) processedCourses.sort((a, b) => { if (a.ambiguous === b.ambiguous) { diff --git a/Closure_Front_End/src/utils.js b/Closure_Front_End/src/utils.js index 7bca3aff..11d97ae4 100644 --- a/Closure_Front_End/src/utils.js +++ b/Closure_Front_End/src/utils.js @@ -1,18 +1,3 @@ - -/** A mapping between API "semester" values to the "semester" property used in the front-end, - which indicates their positioning. - */ -export const API_SEMESTER_TO_PROP_INT = new Map(Object.entries({ - FIRST: 1, - SECOND: 2 -})) - -/** A mapping between the "semester" property values used in the front-end, to their corresponding - * values in the backend API - */ -export const PROP_INT_TO_API_SEMESTER = new Map(Array.from(API_SEMESTER_TO_PROP_INT, a => a.reverse())) - - export function isString(x) { return Object.prototype.toString.call(x) === "[object String]" } diff --git a/Closure_Front_End/src/views/CourseImport.vue b/Closure_Front_End/src/views/CourseImport.vue index d91273ba..70d5a380 100644 --- a/Closure_Front_End/src/views/CourseImport.vue +++ b/Closure_Front_End/src/views/CourseImport.vue @@ -51,7 +51,6 @@ import Instructions from "@/huji-import/Instructions.vue"; import { default as INITIAL_COURSES } from "@/huji-import/example-parsed-courses.js"; import { TRY_HOOK_MESSAGE_TYPE, HOOKED_MESSAGE_TYPE, STARTED_MESSAGE_TYPE, GOT_COURSE_MESSAGE_TYPE, FINISHED_PARSING_MESSAGE_TYPE } from '@/huji-import/course-scrape-communication.js' import ParsedCoursesTables from "@/huji-import/ParsedCoursesTables.vue"; -import { API_SEMESTER_TO_PROP_INT } from "@/utils.js"; const HUJI_ORIGIN = "https://www.huji.ac.il"; @@ -119,12 +118,15 @@ export default { this.courses.push({ ...course, key: this.courses.length }); }, onImportCourses({ courses, importMode }) { - const firstYear = Math.min(...courses.map(course => course.year)); + console.log('onImportCourses', courses) + const firstYear = Math.min(...courses.map(course => course.take.year)); const coursesForDisplay = courses.map(course => { return { ...course, - year: course.year - firstYear + 1, - semester: API_SEMESTER_TO_PROP_INT.get(course.semester) + take: { + year: course.take.year - firstYear + 1, + semester: course.take.semester + } }; }); addCourses(coursesForDisplay, importMode === 'overwrite'); From 2602369f9c807dbc87bcb19041bea3d9c43737d9 Mon Sep 17 00:00:00 2001 From: Daniel Kerbel Date: Tue, 31 Aug 2021 01:07:28 +0300 Subject: [PATCH 23/49] Use 'id' instead of 'pk' in serializers For consistency with model definitions --- Closure_Front_End/src/components/Navigation.vue | 2 +- .../rest_api/serializers/CourseGroupSerializer.py | 2 +- Closure_Project/rest_api/serializers/CourseSerializer.py | 5 ++--- Closure_Project/rest_api/serializers/TrackSerializer.py | 4 ++-- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Closure_Front_End/src/components/Navigation.vue b/Closure_Front_End/src/components/Navigation.vue index e121c6c2..2fecedba 100644 --- a/Closure_Front_End/src/components/Navigation.vue +++ b/Closure_Front_End/src/components/Navigation.vue @@ -58,7 +58,7 @@ export default { return await fetchDjangoListIntoSelectOptions(this.http, url, track => track.name); }, async fetchCourses(query) { - const url = `tracks/${this.selectedTrack?.pk ?? 'null'}/courses/?limit=6&offset=15&data_year=${this.selectedYear}&search=${query}`; + const url = `tracks/${this.selectedTrack?.id ?? 'null'}/courses/?limit=6&offset=15&data_year=${this.selectedYear}&search=${query}`; return await fetchDjangoListIntoSelectOptions(this.http, url, course => course.name); }, } diff --git a/Closure_Project/rest_api/serializers/CourseGroupSerializer.py b/Closure_Project/rest_api/serializers/CourseGroupSerializer.py index 8c8393e2..352c571a 100644 --- a/Closure_Project/rest_api/serializers/CourseGroupSerializer.py +++ b/Closure_Project/rest_api/serializers/CourseGroupSerializer.py @@ -4,7 +4,7 @@ class CourseGroupSerializer(DynamicFieldsModelSerializer): - courses = CourseSerializer(fields=('pk', 'course_id', 'name', 'semester', 'points'), many=True, read_only=True) + courses = CourseSerializer(fields=('id', 'course_id', 'name', 'semester', 'points'), many=True, read_only=True) class Meta: model = CourseGroup diff --git a/Closure_Project/rest_api/serializers/CourseSerializer.py b/Closure_Project/rest_api/serializers/CourseSerializer.py index 412d06ba..5fcc802f 100644 --- a/Closure_Project/rest_api/serializers/CourseSerializer.py +++ b/Closure_Project/rest_api/serializers/CourseSerializer.py @@ -5,12 +5,11 @@ class CourseSerializer(DynamicFieldsModelSerializer): - pk = serializers.PrimaryKeyRelatedField(source='id', read_only=True) - class Meta: model = Course - fields = ('pk', 'course_id', 'data_year', 'name', 'semester', 'points', 'is_given_this_year', + fields = ('id', 'course_id', 'data_year', 'name', 'semester', 'points', 'is_given_this_year', 'is_corner_stone', 'comment') + read_only_fields = ('id',) class CourseOfTrackSerializer(CourseSerializer): type = serializers.SerializerMethodField(method_name='get_type') diff --git a/Closure_Project/rest_api/serializers/TrackSerializer.py b/Closure_Project/rest_api/serializers/TrackSerializer.py index 956ae0e5..c0312d60 100644 --- a/Closure_Project/rest_api/serializers/TrackSerializer.py +++ b/Closure_Project/rest_api/serializers/TrackSerializer.py @@ -4,14 +4,14 @@ class TrackSerializer(DynamicFieldsModelSerializer): - pk = serializers.PrimaryKeyRelatedField(source='id', read_only=True) total_points = serializers.IntegerField(read_only=True) class Meta: model = Track - fields = ('pk', 'track_number', 'name', 'data_year', 'total_points', 'points_must', 'points_from_list', + fields = ('id', 'track_number', 'name', 'data_year', 'total_points', 'points_must', 'points_from_list', 'points_choice', 'points_complementary', 'points_corner_stones', 'points_minor', 'points_additional_hug', 'comment') + read_only_fields = ('id',) class TrackSerializerWithCourseGroups(TrackSerializer): course_groups = CourseGroupSerializer(source='coursegroup_set', many=True, read_only=True) From e48ad4021258479e308fa248a9c6ac1d8d2f1fa1 Mon Sep 17 00:00:00 2001 From: Daniel Kerbel Date: Tue, 31 Aug 2021 03:43:31 +0300 Subject: [PATCH 24/49] Add FE support for saving a course plan --- Closure_Front_End/src/App.vue | 20 ++- Closure_Front_End/src/auth/index.js | 11 +- .../src/components/SaveAsModal.vue | 78 ++++++++++ Closure_Front_End/src/components/SaveMenu.vue | 81 ++++++++++ .../src/components/SaveStateFooter.vue | 49 +++++++ Closure_Front_End/src/course-store.js | 138 ++++++++++++------ 6 files changed, 324 insertions(+), 53 deletions(-) create mode 100644 Closure_Front_End/src/components/SaveAsModal.vue create mode 100644 Closure_Front_End/src/components/SaveMenu.vue create mode 100644 Closure_Front_End/src/components/SaveStateFooter.vue diff --git a/Closure_Front_End/src/App.vue b/Closure_Front_End/src/App.vue index c68e18ef..e5470a56 100644 --- a/Closure_Front_End/src/App.vue +++ b/Closure_Front_End/src/App.vue @@ -6,6 +6,14 @@ Closure()
  • -
  • - -
    {{ track }}
    -
  • - - -
  • - - - -
  • - -
  • - -
    - -
    -
  • - -
  • -
    - -
    -
  • @@ -136,5 +72,4 @@ export default { font-size: 0.75rem; margin: 0.75rem; } - - \ No newline at end of file + \ No newline at end of file diff --git a/Closure_Front_End/src/views/Settings.vue b/Closure_Front_End/src/views/Settings.vue index c0273d30..55ad3d82 100644 --- a/Closure_Front_End/src/views/Settings.vue +++ b/Closure_Front_End/src/views/Settings.vue @@ -6,26 +6,19 @@

    טוען נתונים...

    15% -
    - חלה שגיאה -
    -

    {{error}}

    -
    -
    - -
    +
    ID Claims:
    {{ idClaims }}
    -
    +
    Student Object:
    {{ student }}
    -
    - +
    +
    @@ -33,41 +26,16 @@ - \ No newline at end of file diff --git a/Closure_Front_End/src/components/CourseSelection.vue b/Closure_Front_End/src/components/CourseSelection.vue new file mode 100644 index 00000000..a557140d --- /dev/null +++ b/Closure_Front_End/src/components/CourseSelection.vue @@ -0,0 +1,67 @@ + + + + + \ No newline at end of file diff --git a/Closure_Front_End/src/components/Navigation.vue b/Closure_Front_End/src/components/Navigation.vue index 2ed3bb1e..409d89e2 100644 --- a/Closure_Front_End/src/components/Navigation.vue +++ b/Closure_Front_End/src/components/Navigation.vue @@ -4,66 +4,36 @@
    -
    - +
    +
    -
    - +
    +
    - \ No newline at end of file + \ No newline at end of file diff --git a/Closure_Front_End/src/components/TrackSelection.vue b/Closure_Front_End/src/components/TrackSelection.vue new file mode 100644 index 00000000..f7a5ec1c --- /dev/null +++ b/Closure_Front_End/src/components/TrackSelection.vue @@ -0,0 +1,64 @@ + + + + + \ No newline at end of file diff --git a/Closure_Front_End/src/components/YearSelection.vue b/Closure_Front_End/src/components/YearSelection.vue index 9841ab86..d754f839 100644 --- a/Closure_Front_End/src/components/YearSelection.vue +++ b/Closure_Front_End/src/components/YearSelection.vue @@ -1,27 +1,67 @@ diff --git a/Closure_Front_End/src/utils.js b/Closure_Front_End/src/utils.js index 11d97ae4..ecdcf49b 100644 --- a/Closure_Front_End/src/utils.js +++ b/Closure_Front_End/src/utils.js @@ -29,21 +29,3 @@ export const MODEL_SEMESTER_TO_STRING = new Map([ [1, "ראשון"], [2, "שני"] ]); - -/** - * A function used to query a list endpoint from Django Rest Framework(DRF), - * and return an array of objects suitable for passing into Multiselect component 'options'. - * - * @param {import("axios").AxiosInstance} http used to create requests - * @param {string} url URL of a DRF 'list' retrieval endpoint - * @param {(obj: any) => string} objToLabel Extracts the label(shown in Select) of a model object - * @returns {[any]} Array of multiselect options - */ -export async function fetchDjangoListIntoSelectOptions(http, url, objToLabel) { - console.log('multiselect fetching from', url); - const response = await http.get(url); - const values = response.data.results; - return values.map(value => { return { - value, label: objToLabel(value) - }}); -} diff --git a/Closure_Front_End/src/views/Home.vue b/Closure_Front_End/src/views/Home.vue index 8cafcf4b..6cb56c06 100644 --- a/Closure_Front_End/src/views/Home.vue +++ b/Closure_Front_End/src/views/Home.vue @@ -4,7 +4,7 @@
    - +
    From 9b1776a5ab0f2e3a8d9812a337896214e7f37fdb Mon Sep 17 00:00:00 2001 From: Daniel Kerbel Date: Mon, 6 Sep 2021 20:15:43 +0300 Subject: [PATCH 43/49] Refactor course color to component, use in course selection --- .../src/components/CourseBox.vue | 43 +++----------- .../src/components/CourseColorCircle.vue | 58 +++++++++++++++++++ .../src/components/CourseSelection.vue | 8 ++- 3 files changed, 74 insertions(+), 35 deletions(-) create mode 100644 Closure_Front_End/src/components/CourseColorCircle.vue diff --git a/Closure_Front_End/src/components/CourseBox.vue b/Closure_Front_End/src/components/CourseBox.vue index 439cc586..0e4e2868 100644 --- a/Closure_Front_End/src/components/CourseBox.vue +++ b/Closure_Front_End/src/components/CourseBox.vue @@ -1,9 +1,7 @@ @@ -57,24 +52,4 @@ export default { .fa-times-circle:hover { cursor: pointer; } - -.must { - color: #bc87d0; -} - -.choose_from_list { - color: #fbaf5d; -} - -.corner_stone { - color: #69d85e; -} - -.supplementary { - color: #bfbfbf; -} - -.choice { - color: #f06eaa; -} \ No newline at end of file diff --git a/Closure_Front_End/src/components/CourseColorCircle.vue b/Closure_Front_End/src/components/CourseColorCircle.vue new file mode 100644 index 00000000..21eb7796 --- /dev/null +++ b/Closure_Front_End/src/components/CourseColorCircle.vue @@ -0,0 +1,58 @@ + + + + + \ No newline at end of file diff --git a/Closure_Front_End/src/components/CourseSelection.vue b/Closure_Front_End/src/components/CourseSelection.vue index a557140d..6376b746 100644 --- a/Closure_Front_End/src/components/CourseSelection.vue +++ b/Closure_Front_End/src/components/CourseSelection.vue @@ -19,11 +19,17 @@ + + \ No newline at end of file diff --git a/Closure_Front_End/src/utils.js b/Closure_Front_End/src/utils.js index ecdcf49b..3790cca2 100644 --- a/Closure_Front_End/src/utils.js +++ b/Closure_Front_End/src/utils.js @@ -19,13 +19,14 @@ export const MODEL_COURSE_TYPE_TO_STRING = new Map([ ]); export const MODEL_SEMESTER_TO_STRING = new Map([ - ['FIRST', "ראשון"], - ['SECOND', "שני"], - ['SUMMER', "קיץ"], - ['EITHER', "כל"], - ['ANNUAL', "שנתי"], + ['FIRST', "סמסטר א'"], + ['SECOND', "סמסטר ב'"], + ['SUMMER', "קורס קיץ"], + ['EITHER', "סמסטר א'/ב'"], + ['ANNUAL', "קורס שנתי"], // TODO: standardize model types in the code to avoid this // duplictation - [1, "ראשון"], - [2, "שני"] + [1, "סמסטר א'"], + [2, "סמסטר ב'"] ]); +