From 81ea7844f3fd4e93c1fc0fd98f93afdce52884da Mon Sep 17 00:00:00 2001
From: "robinwilliam.hundt" <robinwilliam.hundt@stud.uni-goettingen.de>
Date: Sun, 11 Mar 2018 19:29:33 +0100
Subject: [PATCH 1/3] Added VisibleCommentFeedbackSerializer
Only Comments that are `visible_to_student=True` will be serialized. For some weird reason i had to resort to a little hack in the serializer, see the comment inside the `get_feedback_lines()` method of the serializer for context.
I choose to not remove Feedback that is not final from the response of the student submissions endpoint (as outlined in #91) and will instead show a message in the frontend. This is easier to implement and potentially better for debugging in the frontend.
---
core/serializers/__init__.py | 3 ++-
core/serializers/feedback.py | 28 +++++++++++++++++++++++-
core/serializers/submission.py | 3 ++-
core/tests/test_student_page.py | 38 ++++++++++++++++++++++++++-------
4 files changed, 61 insertions(+), 11 deletions(-)
diff --git a/core/serializers/__init__.py b/core/serializers/__init__.py
index 0306ce60..1e598184 100644
--- a/core/serializers/__init__.py
+++ b/core/serializers/__init__.py
@@ -1,5 +1,6 @@
from .common_serializers import * # noqa
-from .feedback import FeedbackSerializer, FeedbackCommentSerializer # noqa
+from .feedback import (FeedbackSerializer, FeedbackCommentSerializer,
+ VisibleCommentFeedbackSerializer) # noqa
from .subscription import * # noqa
from .student import * # noqa
from .submission import * # noqa
diff --git a/core/serializers/feedback.py b/core/serializers/feedback.py
index d5e48833..3f854ebd 100644
--- a/core/serializers/feedback.py
+++ b/core/serializers/feedback.py
@@ -62,7 +62,7 @@ class FeedbackCommentDictionarySerializer(serializers.ListSerializer):
return ret
-class FeedbackCommentSerializer(serializers.ModelSerializer):
+class FeedbackCommentSerializer(DynamicFieldsModelSerializer):
of_tutor = serializers.StringRelatedField(source='of_tutor.username')
class Meta:
@@ -179,3 +179,29 @@ class FeedbackSerializer(DynamicFieldsModelSerializer):
model = Feedback
fields = ('pk', 'of_submission', 'is_final', 'score', 'feedback_lines',
'created', 'of_submission_type', 'feedback_stage_for_user')
+
+
+class VisibleCommentFeedbackSerializer(FeedbackSerializer):
+ feedback_lines = serializers.SerializerMethodField()
+ of_submission_type = serializers.ReadOnlyField(
+ source='of_submission.type.pk')
+
+ def get_feedback_lines(self, feedback):
+ comments = feedback.feedback_lines.filter(visible_to_student=True)
+ serializer = FeedbackCommentSerializer(
+ comments,
+ many=True,
+ fields=('pk', 'text', 'created', 'of_line',)
+ )
+ # this is a weird hack because, for some reason, serializer.data
+ # just won't contain the correct data. Instead .data returns a list
+ # containing just the `of_line` attr of the serialized comments
+ # after long debugging i found that for inexplicable reasons
+ # `data.serializer._data` contains the correct data. No clue why.
+ return serializer.data.serializer._data
+
+ class Meta:
+ model = Feedback
+ fields = ('pk', 'of_submission', 'is_final', 'score', 'feedback_lines',
+ 'created', 'of_submission_type')
+
diff --git a/core/serializers/submission.py b/core/serializers/submission.py
index 8286e58b..8e0b1d08 100644
--- a/core/serializers/submission.py
+++ b/core/serializers/submission.py
@@ -2,6 +2,7 @@ from rest_framework import serializers
from core.models import Submission
from core.serializers import (DynamicFieldsModelSerializer, FeedbackSerializer,
+ VisibleCommentFeedbackSerializer,
SubmissionTypeListSerializer,
SubmissionTypeSerializer, TestSerializer)
@@ -18,7 +19,7 @@ class SubmissionNoTextFieldsSerializer(DynamicFieldsModelSerializer):
class SubmissionSerializer(DynamicFieldsModelSerializer):
type = SubmissionTypeSerializer()
- feedback = FeedbackSerializer()
+ feedback = VisibleCommentFeedbackSerializer()
tests = TestSerializer(many=True)
class Meta:
diff --git a/core/tests/test_student_page.py b/core/tests/test_student_page.py
index c8a1365b..fa57aea9 100644
--- a/core/tests/test_student_page.py
+++ b/core/tests/test_student_page.py
@@ -139,9 +139,14 @@ class StudentSelfSubmissionsTests(APITestCase):
'students': [{
'username': 'user01',
}],
- 'tutors': [{
- 'username': 'tutor01'
- }],
+ 'tutors': [
+ {
+ 'username': 'tutor01'
+ },
+ {
+ 'username': 'tutor02'
+ }
+ ],
'submissions': [{
'user': 'user01',
'type': 'problem01',
@@ -150,10 +155,19 @@ class StudentSelfSubmissionsTests(APITestCase):
'text': 'Very bad!',
'score': 3,
'feedback_lines': {
- '1': [{
- 'text': 'This is very bad!',
- 'of_tutor': 'tutor01'
- }],
+ '1': [
+ {
+ 'text': 'This is very bad!',
+ 'of_tutor': 'tutor01',
+ # explicitness to required
+ # will also be set automatically
+ 'visible_to_student': False
+ },
+ {
+ 'text': 'This is good!',
+ 'of_tutor': 'tutor02'
+ }
+ ],
}
}
}]
@@ -210,12 +224,20 @@ class StudentSelfSubmissionsTests(APITestCase):
self.submission_list_first_entry['feedback']['score'],
self.student_info.submissions.first().feedback.score)
- def submssion_feedback_contains_submission_lines(self):
+ def test_submission_feedback_contains_submission_lines(self):
self.assertIn(
'feedback_lines',
self.submission_list_first_entry['feedback']
)
+ def test_feedback_contains_one_comment_per_line(self):
+ lines = self.submission_list_first_entry['feedback']['feedback_lines']
+ self.assertEqual(len(lines[1]), 1)
+
+ def test_feedback_comment_does_not_contain_tutor(self):
+ lines = self.submission_list_first_entry['feedback']['feedback_lines']
+ self.assertNotIn('of_tutor', lines[1][0])
+
# We don't want a matriculation number here
def test_matriculation_number_is_not_send(self):
self.assertNotIn('matrikel_no', self.submission_list_first_entry)
--
GitLab
From 5ba3ed772555c21ecb79478a1cc239ffdd1e46f5 Mon Sep 17 00:00:00 2001
From: "robinwilliam.hundt" <robinwilliam.hundt@stud.uni-goettingen.de>
Date: Sun, 11 Mar 2018 20:22:44 +0100
Subject: [PATCH 2/3] Cleaned up StudentViews / Added test output
closes #91
---
.../components/student/ExamInformation.vue | 2 +-
.../student/NonFinalFeedbackAlert.vue | 23 +++++++++++
.../src/components/student/SubmissionList.vue | 13 +++++-
frontend/src/pages/student/StudentLayout.vue | 7 +---
.../pages/student/StudentSubmissionPage.vue | 40 ++++++++++++-------
5 files changed, 63 insertions(+), 22 deletions(-)
create mode 100644 frontend/src/components/student/NonFinalFeedbackAlert.vue
diff --git a/frontend/src/components/student/ExamInformation.vue b/frontend/src/components/student/ExamInformation.vue
index 21795f05..6ebcf482 100644
--- a/frontend/src/components/student/ExamInformation.vue
+++ b/frontend/src/components/student/ExamInformation.vue
@@ -2,7 +2,7 @@
<table class="table table-info rounded">
<tbody>
<tr>
- <th>Modul</th>
+ <th>Module</th>
<td>{{ exam.module_reference }}</td>
</tr>
<tr>
diff --git a/frontend/src/components/student/NonFinalFeedbackAlert.vue b/frontend/src/components/student/NonFinalFeedbackAlert.vue
new file mode 100644
index 00000000..801c2204
--- /dev/null
+++ b/frontend/src/components/student/NonFinalFeedbackAlert.vue
@@ -0,0 +1,23 @@
+<template>
+ <v-alert type="warning" :value="value" class="non-final-alert ">
+ This feedback is not final! Changes will likely occur!
+ </v-alert>
+</template>
+
+<script>
+ export default {
+ name: 'non-final-feedback-alert',
+ props: {
+ value: {
+ type: Boolean,
+ default: true
+ }
+ }
+ }
+</script>
+
+<style scoped>
+ .non-final-alert {
+ font-weight: bolder;
+ }
+</style>
diff --git a/frontend/src/components/student/SubmissionList.vue b/frontend/src/components/student/SubmissionList.vue
index 916563d5..1c624da3 100644
--- a/frontend/src/components/student/SubmissionList.vue
+++ b/frontend/src/components/student/SubmissionList.vue
@@ -10,7 +10,11 @@
<td>{{ props.item.type.name }}</td>
<td class="text-xs-right">{{ props.item.feedback.score }}</td>
<td class="text-xs-right">{{ props.item.type.full_score }}</td>
- <td class="text-xs-right"><v-btn :to="`/student/submission/${props.item.type.pk}`" color="orange lighten-2"><v-icon>chevron_right</v-icon></v-btn></td>
+ <td class="text-xs-right">
+ <v-btn :to="`/submission/${props.item.type.pk}`" color="orange lighten-2">
+ <v-icon>chevron_right</v-icon>
+ </v-btn>
+ </td>
</template>
</v-data-table>
<v-alert color="info" value="true">
@@ -34,11 +38,18 @@
},
{
text: 'Score',
+ align: 'right',
value: 'feedback.score'
},
{
text: 'Maximum Score',
+ align: 'right',
value: 'type.full_score'
+ },
+ {
+ text: 'View',
+ align: 'center',
+ sortable: false
}
]
}
diff --git a/frontend/src/pages/student/StudentLayout.vue b/frontend/src/pages/student/StudentLayout.vue
index c3100896..02e351af 100644
--- a/frontend/src/pages/student/StudentLayout.vue
+++ b/frontend/src/pages/student/StudentLayout.vue
@@ -25,7 +25,7 @@
v-if="!mini"
class="elevation-1 exam-info ma-1"
/>
- <v-list-tile exact v-for="(item, i) in submissionNavItems" :key="i" :to="item.route">
+ <v-list-tile exact v-for="item in submissionNavItems" :key="item.route" :to="item.route">
<v-list-tile-action>
<v-icon v-if="!visited[item.id]">assignment</v-icon>
<v-icon v-else>check</v-icon>
@@ -54,11 +54,6 @@
name: 'Overview',
icon: 'home',
route: '/home'
- },
- {
- name: 'Statistics',
- icon: 'show_chart',
- route: '/home'
}
]
}
diff --git a/frontend/src/pages/student/StudentSubmissionPage.vue b/frontend/src/pages/student/StudentSubmissionPage.vue
index b751e545..80f6e312 100644
--- a/frontend/src/pages/student/StudentSubmissionPage.vue
+++ b/frontend/src/pages/student/StudentSubmissionPage.vue
@@ -1,5 +1,6 @@
<template>
<v-container flex>
+ <non-final-feedback-alert :value="!feedbackIsFinal"/>
<v-layout row wrap>
<v-flex lg6 md12 mt-5>
<base-annotated-submission>
@@ -19,18 +20,25 @@
</v-toolbar>
<template slot="table-content">
<tr v-for="(code, lineNo) in submission" :key="lineNo">
- <submission-line :code="code" :lineNo="lineNo"/>
- <feedback-comment
- v-if="feedback[lineNo] && showFeedback"
- v-for="(comment, index) in feedback[lineNo]"
- v-bind="comment"
- :key="index"
- />
+ <submission-line :code="code" :lineNo="lineNo">
+ <feedback-comment
+ v-if="feedback[lineNo] && showFeedback"
+ v-for="(comment, index) in feedback[lineNo]"
+ v-bind="comment"
+ :line-no="lineNo"
+ :key="index"
+ />
+ </submission-line>
</tr>
</template>
</base-annotated-submission>
+ <submission-tests
+ :tests="submission.tests"
+ :expand="true"
+ class="mt-3"
+ ></submission-tests>
</v-flex>
- <v-flex lg6 md12>
+ <v-flex lg6 md12 mt-5 pl-3>
<submission-type
v-bind="submissionType">
</submission-type>
@@ -49,10 +57,14 @@
import FeedbackComment from '@/components/submission_notes/base/FeedbackComment'
import {studentPageMut} from '@/store/modules/student-page'
import {subNotesMut} from '@/store/modules/submission-notes'
+ import SubmissionTests from '@/components/SubmissionTests'
+ import NonFinalFeedbackAlert from '@/components/student/NonFinalFeedbackAlert'
export default {
name: 'student-submission-page',
components: {
+ NonFinalFeedbackAlert,
+ SubmissionTests,
FeedbackComment,
SubmissionLine,
BaseAnnotatedSubmission,
@@ -71,10 +83,13 @@
'submission'
]),
...mapState({
- score: function (state) { return state.studentPage.submissionData[this.id].feedback.score },
- submissionType: function (state) { return state.studentPage.submissionData[this.id].type },
- feedback: function (state) {
+ score (state) { return state.studentPage.submissionData[this.id].feedback.score },
+ submissionType (state) { return state.studentPage.submissionData[this.id].type },
+ feedback (state) {
return state.studentPage.submissionData[this.$route.params.id].feedback.feedback_lines
+ },
+ feedbackIsFinal (state) {
+ return state.studentPage.submissionData[this.$route.params.id].feedback.is_final
}
})
},
@@ -87,9 +102,6 @@
},
mounted () {
this.onRouteMountOrUpdate(this.id)
- this.$nextTick(() => {
- window.PR.prettyPrint()
- })
},
beforeRouteUpdate (to, from, next) {
this.onRouteMountOrUpdate(to.params.id)
--
GitLab
From 10f2a6fce40f52156e8c1400e991bc38171adc6a Mon Sep 17 00:00:00 2001
From: "robinwilliam.hundt" <robinwilliam.hundt@stud.uni-goettingen.de>
Date: Sun, 11 Mar 2018 22:35:15 +0100
Subject: [PATCH 3/3] Reviewer can activate/deactivate student access
The reviewer has the option to activate and deactivate all students access via the web interface in the student overview. The corresponding endpoints are additional list routes on the student viewset. Tests are in test_reviewer_viewset.py
---
core/serializers/__init__.py | 2 +-
core/serializers/feedback.py | 1 -
core/serializers/student.py | 4 +-
core/tests/test_student_reviewer_viewset.py | 32 ++++++--
core/views/common_views.py | 20 ++++-
frontend/src/api.js | 8 ++
.../components/student_list/StudentList.vue | 3 +
.../student_list/StudentListMenu.vue | 78 +++++++++++++++++++
8 files changed, 137 insertions(+), 11 deletions(-)
create mode 100644 frontend/src/components/student_list/StudentListMenu.vue
diff --git a/core/serializers/__init__.py b/core/serializers/__init__.py
index 1e598184..20236b11 100644
--- a/core/serializers/__init__.py
+++ b/core/serializers/__init__.py
@@ -1,5 +1,5 @@
from .common_serializers import * # noqa
-from .feedback import (FeedbackSerializer, FeedbackCommentSerializer,
+from .feedback import (FeedbackSerializer, FeedbackCommentSerializer, # noqa
VisibleCommentFeedbackSerializer) # noqa
from .subscription import * # noqa
from .student import * # noqa
diff --git a/core/serializers/feedback.py b/core/serializers/feedback.py
index 3f854ebd..9a4317ff 100644
--- a/core/serializers/feedback.py
+++ b/core/serializers/feedback.py
@@ -204,4 +204,3 @@ class VisibleCommentFeedbackSerializer(FeedbackSerializer):
model = Feedback
fields = ('pk', 'of_submission', 'is_final', 'score', 'feedback_lines',
'created', 'of_submission_type')
-
diff --git a/core/serializers/student.py b/core/serializers/student.py
index d09aade1..efa8c537 100644
--- a/core/serializers/student.py
+++ b/core/serializers/student.py
@@ -22,7 +22,9 @@ class StudentInfoSerializerForListView(DynamicFieldsModelSerializer):
user = serializers.ReadOnlyField(source='user.username')
exam = serializers.ReadOnlyField(source='exam.module_reference')
submissions = SubmissionNoTextFieldsSerializer(many=True)
+ is_active = serializers.BooleanField(source='user.is_active')
class Meta:
model = StudentInfo
- fields = ('pk', 'name', 'user', 'exam', 'submissions', 'matrikel_no')
+ fields = ('pk', 'name', 'user', 'exam', 'submissions',
+ 'matrikel_no', 'is_active')
diff --git a/core/tests/test_student_reviewer_viewset.py b/core/tests/test_student_reviewer_viewset.py
index 9d22d7c1..9e07425e 100644
--- a/core/tests/test_student_reviewer_viewset.py
+++ b/core/tests/test_student_reviewer_viewset.py
@@ -3,6 +3,7 @@ from rest_framework import status
from rest_framework.test import (APIRequestFactory, APITestCase,
force_authenticate)
+from core import models
from core.views import StudentReviewerApiViewSet
from util.factories import make_test_data
@@ -26,11 +27,18 @@ class StudentPageTests(APITestCase):
'description': 'Very hard',
'solution': 'Impossible!'
}],
- 'students': [{
- 'username': 'user01',
- 'fullname': 'us er01',
- 'exam': 'TestExam B.Inf.0042'
- }],
+ 'students': [
+ {
+ 'username': 'user01',
+ 'fullname': 'us er01',
+ 'exam': 'TestExam B.Inf.0042'
+ },
+ {
+ 'username': 'user02',
+ 'exam': 'TestExam B.Inf.0042'
+ }
+
+ ],
'tutors': [{
'username': 'tutor'
}],
@@ -67,7 +75,7 @@ class StudentPageTests(APITestCase):
self.assertEqual(self.response.status_code, status.HTTP_200_OK)
def test_can_see_all_students(self):
- self.assertEqual(1, len(self.response.data))
+ self.assertEqual(2, len(self.response.data))
def test_submissions_score_is_included(self):
self.assertEqual(self.student.submissions.first().feedback.score,
@@ -77,3 +85,15 @@ class StudentPageTests(APITestCase):
print(self.response.data[0]['submissions'][0])
self.assertEqual(self.student.submissions.first().type.full_score,
self.response.data[0]['submissions'][0]['full_score'])
+
+ def test_can_deactivate_all_students(self):
+ self.client.force_authenticate(user=self.reviewer)
+ self.client.post(reverse('student-list') + 'deactivate/')
+ users = [stud.user for stud in models.StudentInfo.objects.all()]
+ self.assertTrue(all([not user.is_active for user in users]))
+
+ def test_can_activate_all_students(self):
+ self.client.force_authenticate(user=self.reviewer)
+ self.client.post(reverse('student-list') + 'activate/')
+ users = [stud.user for stud in models.StudentInfo.objects.all()]
+ self.assertTrue(all([user.is_active for user in users]))
diff --git a/core/views/common_views.py b/core/views/common_views.py
index 8aed85b1..ff4d6abb 100644
--- a/core/views/common_views.py
+++ b/core/views/common_views.py
@@ -5,8 +5,8 @@ import logging
from django.conf import settings
from django.db.models import Avg
-from rest_framework import generics, mixins, viewsets
-from rest_framework.decorators import api_view
+from rest_framework import generics, mixins, viewsets, status
+from rest_framework.decorators import api_view, list_route
from rest_framework.response import Response
from core import models
@@ -63,6 +63,22 @@ class StudentReviewerApiViewSet(viewsets.ReadOnlyModelViewSet):
.all()
serializer_class = StudentInfoSerializerForListView
+ def _set_students_active(self, active):
+ for student in self.get_queryset():
+ user = student.user
+ user.is_active = active
+ user.save()
+
+ @list_route(methods=['post'])
+ def deactivate(self, request):
+ self._set_students_active(False)
+ return Response(status=status.HTTP_200_OK)
+
+ @list_route(methods=['post'])
+ def activate(self, request):
+ self._set_students_active(True)
+ return Response(status=status.HTTP_200_OK)
+
class ExamApiViewSet(viewsets.ReadOnlyModelViewSet):
""" Gets a list of an individual exam by Id if provided """
diff --git a/frontend/src/api.js b/frontend/src/api.js
index 0e040c64..26799ada 100644
--- a/frontend/src/api.js
+++ b/frontend/src/api.js
@@ -167,4 +167,12 @@ export async function patchComment (comment = {pk: undefined}) {
return (await ax.patch(url, comment)).data
}
+export async function activateAllStudentAccess () {
+ return ax.post('/api/student/activate/')
+}
+
+export async function deactivateAllStudentAccess () {
+ return ax.post('/api/student/deactivate/')
+}
+
export default ax
diff --git a/frontend/src/components/student_list/StudentList.vue b/frontend/src/components/student_list/StudentList.vue
index cdca5006..fd8e2e16 100644
--- a/frontend/src/components/student_list/StudentList.vue
+++ b/frontend/src/components/student_list/StudentList.vue
@@ -14,6 +14,7 @@
></v-text-field>
<v-card-actions>
<v-btn icon @click="refresh"><v-icon>refresh</v-icon></v-btn>
+ <student-list-menu/>
</v-card-actions>
</v-card-title>
<v-data-table
@@ -98,8 +99,10 @@
<script>
import {mapActions, mapState} from 'vuex'
+ import StudentListMenu from '@/components/student_list/StudentListMenu'
export default {
+ components: {StudentListMenu},
name: 'student-list',
data () {
return {
diff --git a/frontend/src/components/student_list/StudentListMenu.vue b/frontend/src/components/student_list/StudentListMenu.vue
new file mode 100644
index 00000000..a1a2c92f
--- /dev/null
+++ b/frontend/src/components/student_list/StudentListMenu.vue
@@ -0,0 +1,78 @@
+<template>
+ <v-menu open-on-hover bottom offset-y>
+ <v-btn icon slot="activator">
+ <v-icon>menu</v-icon>
+ </v-btn>
+ <v-list>
+ <v-list-tile v-for="item in items" :key="item.title" @click="item.action">
+ <v-list-tile-title>{{ item.title }}</v-list-tile-title>
+ </v-list-tile>
+ </v-list>
+ </v-menu>
+</template>
+
+<script>
+ import {activateAllStudentAccess,
+ deactivateAllStudentAccess} from '@/api'
+
+ export default {
+ name: 'student-list-menu',
+ computed: {
+ studentsActive () {
+ const firstStudent = Object.values(this.$store.state.students)[0]
+ return firstStudent ? firstStudent.is_active === true : false
+ },
+ items () {
+ return [
+ {
+ title: this.studentsActive
+ ? 'Deactivate student access'
+ : 'Activate student access',
+ action: this.changeStudentsAccess
+ }
+ ]
+ }
+ },
+ methods: {
+ updateStudentData (fields = []) {
+ this.$store.dispatch('getStudents', {
+ studentPks: Object.keys(this.$store.state.students),
+ fields
+ }).catch(() => {
+ this.$notify({
+ title: 'ERROR',
+ text: 'Unable to update student data!',
+ type: 'error'
+ })
+ })
+ },
+ changeStudentsAccess () {
+ if (this.studentsActive) {
+ deactivateAllStudentAccess().then(() => {
+ this.updateStudentData()
+ }).catch(() => {
+ this.$notify({
+ title: 'ERROR',
+ text: 'Unable to disable access',
+ type: 'error'
+ })
+ })
+ } else {
+ activateAllStudentAccess().then(() => {
+ this.updateStudentData()
+ }).catch(() => {
+ this.$notify({
+ title: 'ERROR',
+ text: 'Unable to activate access',
+ type: 'error'
+ })
+ })
+ }
+ }
+ }
+ }
+</script>
+
+<style scoped>
+
+</style>
--
GitLab