FeedbackHistory can now be filtered by labels

Also in the Feedbackhistory it is now possible to filter by excluding labels.
This way, a user can filter for all Submissions did not compile but were not empty

core/migrations/0010_auto_20180805_1139.py

@@ -16,7 +16,7 @@ class Migration(migrations.Migration):
             name='useraccount',
             managers=[
                 ('objects', django.contrib.auth.models.UserManager()),
-                ('tutors', core.models.TutorManager()),
+                ('tutors', core.models.TutorReviewerManager()),
             ],
         ),
     ]

core/migrations/0017_auto_20190604_1631.py

+# Generated by Django 2.1.4 on 2019-06-04 16:31
+
+import core.models.user_account
+import django.contrib.auth.models
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('core', '0016_auto_20190521_1803'),
+    ]
+
+    operations = [
+        migrations.AlterModelManagers(
+            name='useraccount',
+            managers=[
+                ('objects', django.contrib.auth.models.UserManager()),
+                ('corrector', core.models.user_account.TutorReviewerManager()),
+            ],
+        ),
+    ]

core/models/__init__.py

 from .exam_type import ExamType  # noqa
 from .submission_type import SubmissionType  # noqa
-from .user_account import UserAccount, TutorManager  # noqa
+from .user_account import UserAccount, TutorReviewerManager  # noqa
 from .student_info import StudentInfo, random_matrikel_no  # noqa
 from .test import Test  # noqa
 from .submission import Submission, MetaSubmission  # noqa

core/models/label.py

@@ -14,7 +14,7 @@ HexColourValidator = RegexValidator(
 
 
 class FeedbackLabel(models.Model):
-    name = models.CharField(max_length=50)
+    name = models.CharField(max_length=50, unique=True)
     description = models.TextField()
     colour = models.CharField(validators=[HexColourValidator], max_length=7, default='#b0b0b0')
     feedback = models.ManyToManyField(Feedback, related_name='labels')

core/models/user_account.py

@@ -12,10 +12,11 @@ log = logging.getLogger(__name__)
 config = constance.config
 
 
-class TutorManager(UserManager):
+class TutorReviewerManager(UserManager):
 
     def get_queryset(self):
-        return super().get_queryset().filter(role=UserAccount.TUTOR)
+        return super().get_queryset().filter(
+            Q(role=UserAccount.TUTOR) | Q(role=UserAccount.REVIEWER))
 
     def with_feedback_count(self):
         def _get_counter(stage):
@@ -64,7 +65,7 @@ class UserAccount(AbstractUser):
 
     # Managers
     objects = UserManager()
-    tutors = TutorManager()
+    corrector = TutorReviewerManager()
 
     # Helper methods
     def is_student(self):

core/serializers/__init__.py

@@ -4,5 +4,5 @@ from .feedback import (FeedbackSerializer, FeedbackCommentSerializer,  # noqa
 from .subscription import *  # noqa
 from .student import *  # noqa
 from .submission import *  # noqa
-from .tutor import TutorSerializer  # noqa
+from .tutor import CorrectorSerializer  # noqa
 from .label import LabelSerializer  # noqa

core/serializers/tutor.py

@@ -13,7 +13,7 @@ log = logging.getLogger(__name__)
 user_factory = GradyUserFactory()
 
 
-class TutorSerializer(DynamicFieldsModelSerializer):
+class CorrectorSerializer(DynamicFieldsModelSerializer):
     feedback_created = serializers.SerializerMethodField()
     feedback_validated = serializers.SerializerMethodField()
     password = serializers.CharField(

core/tests/test_access_rights.py

@@ -4,7 +4,7 @@ from rest_framework.test import (APIRequestFactory, APITestCase,
                                  force_authenticate)
 
 from core.views import (ExamApiViewSet, StudentReviewerApiViewSet,
-                        StudentSelfApiView, TutorApiViewSet)
+                        StudentSelfApiView, CorrectorApiViewSet)
 from util.factories import GradyUserFactory, make_exams
 
 
@@ -67,7 +67,7 @@ class AccessRightsOfTutorAPIViewTests(APITestCase):
         self.tutor = self.user_factory.make_tutor()
         self.reviewer = self.user_factory.make_reviewer()
         self.request = self.factory.get(reverse('tutor-list'))
-        self.view = TutorApiViewSet.as_view({'get': 'list'})
+        self.view = CorrectorApiViewSet.as_view({'get': 'list'})
 
     def test_unauthenticated_access_denied(self):
         response = self.view(self.request)

core/tests/test_export.py

@@ -118,7 +118,9 @@ class ExportInstanceTest(APITestCase):
         self.assertIn('pk', instance['students'][0])
         self.assertIn('userPk', instance['students'][0])
         self.assertIn('exam', instance['students'][0])
-        self.assertEqual('student01', instance['students'][1]['user'])
+        student_users = [s['user'] for s in instance['students']]
+        self.assertIn('student01', student_users)
+        self.assertIn('student02', student_users)
         self.assertLess(0, len(instance['students'][1]['submissions']))
 
         # students[submissions] nested
@@ -155,7 +157,9 @@ class ExportInstanceTest(APITestCase):
         # tutors fields
         self.assertIn('tutors', instance)
         self.assertLess(0, len(instance['tutors']))
-        self.assertEqual('tutor01', instance['tutors'][0]['username'])
+        tutor_names = [t['username'] for t in instance['tutors']]
+        self.assertIn('tutor01', tutor_names)
+        self.assertIn('reviewer', tutor_names)
 
 
 class ExportJSONTest(APITestCase):

core/tests/test_tutor_api_endpoints.py

@@ -14,7 +14,7 @@ import os
 
 from core.models import (Feedback, SubmissionSubscription,
                          TutorSubmissionAssignment)
-from core.views import TutorApiViewSet
+from core.views import CorrectorApiViewSet
 from util.factories import GradyUserFactory, make_test_data
 
 NUMBER_OF_TUTORS = 3
@@ -32,7 +32,7 @@ class TutorDeleteTest(APITestCase):
         self.reviewer = self.user_factory.make_reviewer()
         self.request = self.factory.delete(reverse('tutor-detail',
                                                    args=[str(self.tutor.pk)]))
-        self.view = TutorApiViewSet.as_view({'delete': 'destroy'})
+        self.view = CorrectorApiViewSet.as_view({'delete': 'destroy'})
 
         force_authenticate(self.request, user=self.reviewer)
         self.response = self.view(self.request, pk=str(self.tutor.pk))
@@ -53,7 +53,7 @@ class TutorListTests(APITestCase):
         factory = APIRequestFactory()
 
         request = factory.get(reverse('tutor-list'))
-        view = TutorApiViewSet.as_view({'get': 'list'})
+        view = CorrectorApiViewSet.as_view({'get': 'list'})
 
         data = make_test_data(data_dict={
             'exams': [{
@@ -121,8 +121,8 @@ class TutorListTests(APITestCase):
     def test_can_access(self):
         self.assertEqual(self.response.status_code, status.HTTP_200_OK)
 
-    def test_get_a_list_of_all_tutors(self):
-        self.assertEqual(2, len(self.response.data))
+    def test_get_a_list_of_all_correctos(self):
+        self.assertEqual(3, len(self.response.data))
 
     def test_feedback_created_count_matches_database(self):
         def verify_fields(tutor_obj):
@@ -169,7 +169,7 @@ class TutorCreateTests(APITestCase):
         self.reviewer = self.user_factory.make_reviewer()
         self.request = self.factory.post(reverse('tutor-list'),
                                          {'username': self.USERNAME})
-        self.view = TutorApiViewSet.as_view({'post': 'create'})
+        self.view = CorrectorApiViewSet.as_view({'post': 'create'})
 
         force_authenticate(self.request, user=self.reviewer)
         self.response = self.view(self.request, username=self.USERNAME)
@@ -218,7 +218,7 @@ class TutorRegisterTests(APITestCase):
     @pytest.mark.skipif(os.environ.get('DJANGO_DEV', False),
                         reason="No password strengths checks in dev")
     def test_password_is_strong_enough(self):
-        response = self.client.post('/api/tutor/register/', {
+        response = self.client.post('/api/corrector/register/', {
             'username': 'hans',
             'password': 'weak'
         })
@@ -227,7 +227,7 @@ class TutorRegisterTests(APITestCase):
         self.assertIn('password', response.data)
 
     def test_anonymous_can_request_access(self):
-        response = self.client.post('/api/tutor/register/', {
+        response = self.client.post('/api/corrector/register/', {
             'username': 'hans',
             'password': 'safeandsound'
         })
@@ -235,7 +235,7 @@ class TutorRegisterTests(APITestCase):
         self.assertEqual(status.HTTP_201_CREATED, response.status_code)
 
     def test_cannot_register_active(self):
-        response = self.client.post('/api/tutor/register/', {
+        response = self.client.post('/api/corrector/register/', {
             'username': 'hans',
             'password': 'safeandsound',
             'is_active': True
@@ -244,7 +244,7 @@ class TutorRegisterTests(APITestCase):
         self.assertEqual(status.HTTP_403_FORBIDDEN, response.status_code)
 
     def test_reviewer_can_activate_tutor(self):
-        response = self.client.post('/api/tutor/register/', {
+        response = self.client.post('/api/corrector/register/', {
             'username': 'hans',
             'password': 'safeandsound'
         })
@@ -252,7 +252,7 @@ class TutorRegisterTests(APITestCase):
         self.assertEqual(status.HTTP_201_CREATED, response.status_code)
 
         self.client.force_authenticate(self.reviewer)
-        response = self.client.put('/api/tutor/%s/' % response.data['pk'], {
+        response = self.client.put('/api/corrector/%s/' % response.data['pk'], {
             'username': 'hans',
             'is_active': True
         })
@@ -260,10 +260,10 @@ class TutorRegisterTests(APITestCase):
         self.assertEqual(status.HTTP_200_OK, response.status_code)
 
     def test_trottle_is_not_active_while_testing(self):
-        r = self.client.post('/api/tutor/register/', {'username': 'hans'})
-        r = self.client.post('/api/tutor/register/', {'username': 'the'})
-        r = self.client.post('/api/tutor/register/', {'username': 'brave'})
-        r = self.client.post('/api/tutor/register/', {'username': 'fears'})
-        r = self.client.post('/api/tutor/register/', {'username': 'spiders'})
+        r = self.client.post('/api/corrector/register/', {'username': 'hans'})
+        r = self.client.post('/api/corrector/register/', {'username': 'the'})
+        r = self.client.post('/api/corrector/register/', {'username': 'brave'})
+        r = self.client.post('/api/corrector/register/', {'username': 'fears'})
+        r = self.client.post('/api/corrector/register/', {'username': 'spiders'})
 
         self.assertNotEqual(status.HTTP_429_TOO_MANY_REQUESTS, r.status_code)

core/urls.py

@@ -16,7 +16,7 @@ router.register('feedback-comment', views.FeedbackCommentApiView)
 router.register('submission', views.SubmissionViewSet,
                 basename='submission')
 router.register('submissiontype', views.SubmissionTypeApiView)
-router.register('tutor', views.TutorApiViewSet, basename='tutor')
+router.register('corrector', views.CorrectorApiViewSet, basename='tutor')
 router.register('subscription', views.SubscriptionApiViewSet,
                 basename='subscription')
 router.register('assignment', views.AssignmentApiViewSet)

core/views/common_views.py

@@ -23,7 +23,7 @@ from core.permissions import IsReviewer, IsStudent, IsTutorOrReviewer
 from core.serializers import (ExamSerializer, StudentInfoSerializer,
                               StudentInfoForListViewSerializer,
                               SubmissionNoTypeSerializer, StudentSubmissionSerializer,
-                              SubmissionTypeSerializer, TutorSerializer,
+                              SubmissionTypeSerializer, CorrectorSerializer,
                               UserAccountSerializer)
 
 log = logging.getLogger(__name__)
@@ -96,7 +96,7 @@ class ExamApiViewSet(viewsets.ReadOnlyModelViewSet):
     serializer_class = ExamSerializer
 
 
-class TutorApiViewSet(
+class CorrectorApiViewSet(
         mixins.RetrieveModelMixin,
         mixins.UpdateModelMixin,
         mixins.CreateModelMixin,
@@ -105,11 +105,11 @@ class TutorApiViewSet(
         viewsets.GenericViewSet):
     """ Api endpoint for creating, listing, viewing or deleting tutors """
     permission_classes = (IsReviewer,)
-    queryset = models.UserAccount.tutors \
+    queryset = models.UserAccount.corrector \
         .with_feedback_count() \
         .prefetch_related('subscriptions') \
         .prefetch_related('subscriptions__assignments')
-    serializer_class = TutorSerializer
+    serializer_class = CorrectorSerializer
 
     @action(detail=False, methods=['post'], permission_classes=[AllowAny])
     @throttle_classes([AnonRateThrottle])

core/views/export.py

@@ -10,7 +10,7 @@ from core.permissions import IsReviewer
 from core.serializers.common_serializers import SubmissionTypeSerializer, \
     ExamSerializer, UserAccountSerializer
 from core.serializers.student import StudentExportSerializer
-from core.serializers.tutor import TutorSerializer
+from core.serializers.tutor import CorrectorSerializer
 
 words = xp.generate_wordlist(wordfile=xp.locate_wordfile(), min_length=5, max_length=8)
 
@@ -59,7 +59,9 @@ class InstanceExport(APIView):
         exam_types_serializer = ExamSerializer(ExamType.objects.all(), many=True)
         submission_types_serializer = SubmissionTypeSerializer(
             SubmissionType.objects.all(), many=True)
-        tutors_serializer = TutorSerializer(UserAccount.tutors.with_feedback_count(), many=True)
+        tutors_serializer = CorrectorSerializer(
+            UserAccount.corrector.with_feedback_count(),
+            many=True)
         reviewer_serializer = UserAccountSerializer(UserAccount.get_reviewers(), many=True)
         student_serializer = StudentExportSerializer(StudentInfo.objects.all(), many=True)
 

frontend/src/api.ts

@@ -34,7 +34,7 @@ let ax: AxiosInstance = axios.create({
 }
 
 export async function registerTutor (credentials: Credentials): Promise<AxiosResponse<Tutor>> {
-  return ax.post<Tutor>('/api/tutor/register/', credentials)
+  return ax.post<Tutor>('/api/corrector/register/', credentials)
 }
 
 export async function fetchJWT (credentials: Credentials): Promise<JSONWebToken> {
@@ -77,7 +77,7 @@ export async function fetchStudent ({ pk }:
 }
 
 export async function fetchAllTutors (): Promise<Array<Tutor>> {
-  const url = '/api/tutor/'
+  const url = '/api/corrector/'
   return (await ax.get(url)).data
 }
 

frontend/src/components/LabelStatistics.vue

@@ -6,10 +6,11 @@
       :pagination="pagination"
       :loading="loading"
       :items="summedLabelCounts"
+      hide-actions
     >
       <template slot="items" slot-scope="props">
-        <td>{{ props.item[0] }}</td>
-        <td class="text-xs-center">{{ props.item[1] }}</td>
+        <td>{{ props.item.name }}</td>
+        <td class="text-xs-center">{{ props.item.count }}</td>
       </template>
     </v-data-table>
 
@@ -19,13 +20,14 @@
       </v-card-title>
       <v-data-table
       :headers=headers
-      :pagination="pagination"
+      :pagination.sync="pagination"
       :loading="loading"
       :items="labelCounts"
+      hide-actions
     >
       <template slot="items" slot-scope="props">
-        <td>{{ props.item[0] }}</td>
-        <td class="text-xs-center">{{ props.item[1] }}</td>
+        <td>{{ props.item.name }}</td>
+        <td class="text-xs-center">{{ props.item.count }}</td>
       </template>
     </v-data-table>
 
@@ -50,7 +52,7 @@ export default class LabelStatistics extends Vue{
 
   pagination = {
     descending: true,
-    sortBy: '[1]',
+    sortBy: 'count',
     rowsPerPage: -1
   }
 
@@ -58,14 +60,12 @@ export default class LabelStatistics extends Vue{
     {
       text: 'Label',
       align: 'left',
-      sortable: true,
-      value: '[0]'
+      value: 'name'
     },
     {
       text: 'Count',
       align: 'center',
-      sortable: true,
-      value: '[1]'
+      value: 'count'
     }
   ]
 
@@ -99,7 +99,8 @@ export default class LabelStatistics extends Vue{
         .filter(([key, val]) => key !== 'pk')
       const subTypeName = getters.submissionType(labelStatistics.pk).name
       return [subTypeName, this.mapLabelList(labelValues)]
-    })
+      // it seems the typechecker has a bug here...
+    }).sort((a: any, b: any) => a[0].localeCompare(b[0]))
   }
 
   mapLabelList (labelList: [string, number][]) {
@@ -108,7 +109,7 @@ export default class LabelStatistics extends Vue{
           return String(label.pk) === entry[0]
         })
         const labelName = label ? label.name : 'Unknown label'
-        return [labelName, entry[1]]
+        return {name: labelName, count: entry[1]}
       })
   }
 

frontend/src/components/feedback_list/FeedbackListHelpCard.vue

 <template>
   <v-layout justify-center>
-    <v-card class="mt-5">
+    <v-card class="mt-5" v-if="isReviewer">
+      <v-card-title class="title">
+        This is the history of all the feedback!
+      </v-card-title>
+      <v-card-text>
+        To the left you see all the submissions everyone corrected including the score, final status, etc. You can do the following:<br><br>
+        <ol style="padding-left: 30px;">
+          <li>click on one of the rows to see the submission, including the feedback.</li>
+          <li>sort the table via clicking on the table headers</li>
+          <li>search the type names and also the <b>content</b> of the feedback that was written<br/>
+            (e.g. you're looking for a feedback where someone mentioned a segmentation fault, just type it into the search!)
+          </li>
+          <li>filter by assigned labels</li>
+          <li>filter by the tutors that worked on a submission</li>
+        </ol>
+      </v-card-text>
+    </v-card>
+    <v-card v-else>
       <v-card-title class="title">
         This is the history of your feedback!
       </v-card-title>
@@ -21,9 +38,13 @@
 <script lang="ts">
 import Vue from 'vue'
 import Component from 'vue-class-component'
+import { Authentication } from '../../store/modules/authentication';
 
 @Component
 export default class FeedbackListHelpCard extends Vue {
+  get isReviewer () {
+    return Authentication.isReviewer
+  }
 }
 </script>
 

frontend/src/components/feedback_list/FeedbackSearchOptions.vue

@@ -35,6 +35,34 @@
         </v-tooltip>
       </v-layout>
     </v-flex>
+    <v-container pa-0>
+      <v-layout row>
+        <v-flex md5>
+          <v-select
+            label="Label"
+            :items="labels"
+            v-model="filterByLabels"
+            multiple
+            hint="Filter by label"
+            persistent-hint
+            clearable
+          />
+        </v-flex>
+        <v-flex md5 offset-md1>
+          <v-select
+            label="Exclude label"
+            :items="labels"
+            v-model="filterByExcludingLabels"
+            multiple
+            hint="Filter by excluding labels"
+            persistent-hint
+            clearable
+          />
+        </v-flex>
+      </v-layout>
+    </v-container>
+    <v-container pa-0>
+      <v-layout row>
         <v-flex md12 lg5  v-if="isReviewer">
           <v-select
             label="Tutors"
@@ -58,6 +86,8 @@
           />
         </v-flex>
       </v-layout>
+    </v-container>
+  </v-layout>
 </template>
 
 <script lang="ts">
@@ -70,6 +100,7 @@ import { Authentication } from '@/store/modules/authentication'
 import { actions } from '@/store/actions'
 import { getters } from '@/store/getters'
 import { TutorOverview } from '@/store/modules/tutor-overview'
+import { FeedbackLabels } from '../../store/modules/feedback-labels';
 
 @Component
 export default class FeedbackSearchOptions extends Vue {
@@ -82,12 +113,19 @@ export default class FeedbackSearchOptions extends Vue {
     return this.tutors.map(tutor => tutor.username)
   }
 
+  get labels () {
+    return FeedbackLabels.state.labels.map(label => label.name)
+  }
+
   get showFinal () { return SearchOptions.state.showFinal }
   get searchOtherUserComments () { return SearchOptions.state.searchOtherUserComments }
   get caseSensitive () { return SearchOptions.state.caseSensitive }
   get useRegex () { return SearchOptions.state.useRegex }
   get filterByTutors () { return SearchOptions.state.filterByTutors }
   get filterByStage () { return SearchOptions.state.filterByStage }
+  get filterByLabels () { return SearchOptions.state.filterByLabels }
+  get filterByExcludingLabels () { return SearchOptions.state.filterByExcludingLabels }
+
 
   set showFinal (val) {
     SearchOptions.SET_SHOW_FINAL(val)
@@ -107,6 +145,12 @@ export default class FeedbackSearchOptions extends Vue {
   set filterByStage (val) {
     SearchOptions.SET_FILTER_BY_STAGE(val)
   }
+  set filterByLabels (val) {
+    SearchOptions.SET_FILTER_BY_LABELS(val)
+  }
+  set filterByExcludingLabels (val) {
+    SearchOptions.SET_FILTER_BY_EXCLUDING_LABELS(val)
+  }
 
   loadTutors () {
     if (this.tutors.length === 0 && this.isReviewer) {

frontend/src/components/feedback_list/FeedbackTable.vue

@@ -62,13 +62,16 @@ export default class FeedbackTable extends Vue {
   get useRegex () { return OptionsModule.state.useRegex }
   get filterByTutors () { return OptionsModule.state.filterByTutors }
   get filterByStage () { return OptionsModule.state.filterByStage }
+  get filterByLabels () { return OptionsModule.state.filterByLabels }
   get isTutor () { return Authentication.isTutor }
 
   get stageFilterString () { return OptionsModule.stageFilterString }
 
   get feedback () {
     return Object.values(FeedbackModule.state.feedbackHist).filter(feedback => {
-      return this.checkFinal(feedback) && this.filterFeedbackByTutorStage(feedback)
+      return this.checkFinal(feedback) &&
+        this.filterFeedbackByTutorStage(feedback) &&
+        this.filterFeedbackByLabels(feedback)
     })
   }
 
@@ -106,6 +109,44 @@ export default class FeedbackTable extends Vue {
     return this.filterByTutors.length === 0 ||
         associatedTutors.some(tutor => this.filterByTutors.includes(tutor))
   }
+
+  extractLabelsFromFeedback(feedback: FeedbackHistoryItem): Set<number> {
+    let labels: Set<number> = new Set(feedback.labels)
+
+    Object.values(feedback.feedbackLines || {}).forEach(comments => {
+      comments.forEach(comment => {
+        if (!comment.visibleToStudent) {
+          return
+        }
+        comment.labels.forEach(label => {
+          labels.add(label)
+        })
+      })
+    })
+
+    return labels
+  }
+
+  filterFeedbackByLabels (feedback: FeedbackHistoryItem): boolean {
+    if (this.filterByLabels.length == 0) {
+      return true
+    }
+    const labelsInFeedback = this.extractLabelsFromFeedback(feedback)
+
+    const containsFilterLabels = OptionsModule.labelFilterMapped.every(label => {
+      // ignore if a mapped label is undefined
+      return !label || labelsInFeedback.has(label.pk)
+    })
+
+    const excludedLabels = OptionsModule.labelExcludeFilterMapped
+
+    const containsExcludedLabels = excludedLabels.length && excludedLabels.every(label => {
+      return !label || labelsInFeedback.has(label.pk)
+    })
+
+    return containsFilterLabels && !containsExcludedLabels
+  }
+
   showSubmission (submissionPk: string) {
     this.$router.push(`/feedback/${submissionPk}`)
   }

frontend/src/components/student_list/StudentList.vue

@@ -103,7 +103,7 @@
       </template>
     </v-data-table>
   </v-card>
-</template>
+</template>youyou
 
 <script>
 import { mapActions, mapState } from 'vuex'

frontend/src/pages/base/TutorReviewerBaseLayout.vue

@@ -55,7 +55,7 @@ export default {
         {
           name: 'Statistics',
           icon: 'bar_chart',
-          route: 'statistics'
+          route: '/statistics'
         }
       ]
     }