diff --git a/core/serializers/tutor.py b/core/serializers/tutor.py index ce893d09968383b407e1ff04519257a1ab9b0fa0..6c7e116dd82a2961c32d7bf262c0bf26bbc26db0 100644 --- a/core/serializers/tutor.py +++ b/core/serializers/tutor.py @@ -21,6 +21,7 @@ class CorrectorSerializer(DynamicFieldsModelSerializer): write_only=True, required=False ) + role = serializers.CharField(read_only=True) def get_feedback_created(self, t): ''' It is required that this field was previously annotated ''' @@ -58,4 +59,5 @@ class CorrectorSerializer(DynamicFieldsModelSerializer): 'username', 'feedback_created', 'feedback_validated', - 'exercise_groups') + 'exercise_groups', + 'role') \ No newline at end of file diff --git a/core/tests/test_user_account_views.py b/core/tests/test_user_account_views.py index a26adb97639f15a4025f87996a3948f4ca8ad880..e64beb3c235e2bfee0ee6be58795befb5f63f6bd 100644 --- a/core/tests/test_user_account_views.py +++ b/core/tests/test_user_account_views.py @@ -94,3 +94,77 @@ class TutorReviewerCanChangePasswordTests(APITestCase): self.client.force_authenticate(user=self.reviewer) res = self.client.patch(url, data) self.assertEqual(status.HTTP_403_FORBIDDEN, res.status_code) + + +class ReviewerCanChangeCorrectorRoleTests(APITestCase): + @classmethod + def setUpTestData(cls): + cls.user_factory = GradyUserFactory() + + def setUp(self): + self.reviewer1 = self.user_factory.make_reviewer() + self.client = APIClient() + + def _set_role(self, new_value, changing_user, user_to_change): + self.client.force_authenticate(user=changing_user) + url = f"/api/user/{user_to_change.pk}/change_role/" + return self.client.patch(url, data={'role': new_value}) + + def _make_reviewer(self, changing_user, user_to_change): + return self._set_role('Reviewer', changing_user, user_to_change) + + def _make_tutor(self, changing_user, user_to_change): + return self._set_role('Tutor', changing_user, user_to_change) + + def test_reviewer_can_promote_tutor_to_reviewer(self): + tutor = self.user_factory.make_tutor() + response = self._make_reviewer(self.reviewer1, tutor) + self.assertEqual(response.status_code, status.HTTP_200_OK) + tutor.refresh_from_db() + self.assertTrue(tutor.is_reviewer()) + + def test_reviewer_can_demote_other_reviewer_to_tutor(self): + reviewer2 = self.user_factory.make_reviewer() + response = self._make_tutor(self.reviewer1, reviewer2) + self.assertEqual(response.status_code, status.HTTP_200_OK) + reviewer2.refresh_from_db() + self.assertFalse(reviewer2.is_reviewer()) + + def test_reviewer_cannot_promote_student_to_reviewer(self): + exam = make_exams(exams=[{ + 'module_reference': 'Test Exam 01', + 'total_score': 100, + 'pass_score': 60, + }])[0] + student = self.user_factory.make_student(exam=exam) + response = self._make_reviewer(self.reviewer1, student) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_reviewer_cannot_promote_student_to_tutor(self): + exam = make_exams(exams=[{ + 'module_reference': 'Test Exam 01', + 'total_score': 100, + 'pass_score': 60, + }])[0] + student = self.user_factory.make_student(exam=exam) + response = self._make_tutor(self.reviewer1, student) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_student_cannot_change_access_rights(self): + exam = make_exams(exams=[{ + 'module_reference': 'Test Exam 01', + 'total_score': 100, + 'pass_score': 60, + }])[0] + student = self.user_factory.make_student(exam=exam) + response = self._make_reviewer(student, self.reviewer1) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_tutor_cannot_change_access_rights(self): + tutor = self.user_factory.make_tutor() + response = self._make_reviewer(tutor, self.reviewer1) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_reviewer_cannot_demote_self_to_tutor(self): + response = self._make_tutor(self.reviewer1, self.reviewer1) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) diff --git a/core/views/common_views.py b/core/views/common_views.py index fa33a262f68defa210a26cf510ac9fe3a894b67e..0b6fcbe1855f79d358802ea3dfb97d877db92dd1 100644 --- a/core/views/common_views.py +++ b/core/views/common_views.py @@ -353,6 +353,34 @@ class UserAccountViewSet(viewsets.ReadOnlyModelViewSet): user = self.get_object() return Response(user.exercise_groups, status=status.HTTP_200_OK) + @action(detail=True, methods=["patch"]) + def change_role(self, request, *args, **kwargs): + new_role = request.data.get('role') + user = self.get_object() + valid_values = [ + models.UserAccount.STUDENT, + models.UserAccount.REVIEWER, + models.UserAccount.TUTOR, + ] + if new_role not in valid_values: + error_msg = ( + "You need to provide a 'role' field with one of these values: " + + ', '.join(valid_values) + ) + return Response({'Error': error_msg}, status.HTTP_400_BAD_REQUEST) + if not request.user.is_reviewer(): + error_msg = 'Only reviewers can manage access rights.' + return Response({'Error': error_msg}, status.HTTP_403_FORBIDDEN) + if user.is_student(): + error_msg = 'Cannot promote a student to another role.' + return Response({'Error': error_msg}, status.HTTP_403_FORBIDDEN) + if user == request.user and not new_role == models.UserAccount.REVIEWER: + error_msg = 'As a reviewer, you cannot demote yourself.' + return Response({'Error': error_msg}, status.HTTP_403_FORBIDDEN) + user.role = new_role + user.save() + return Response(status.HTTP_200_OK) + @action(detail=False) def me(self, request): serializer = self.get_serializer(request.user) diff --git a/frontend/src/api.ts b/frontend/src/api.ts index 4ed1c8f67a3ca286cd46a786661b42bdaa148bc6..6ecaedaf34c3edcc84f0596ff0a5ddc4e56b27c3 100644 --- a/frontend/src/api.ts +++ b/frontend/src/api.ts @@ -222,6 +222,10 @@ export async function changeActiveForUser (userPk: string, active: boolean): Pro return (await ax.patch(`/api/user/${userPk}/change_active/`, { 'is_active': active })).data } +export async function changeUserRole (userPk: string, role: UserAccount.RoleEnum): Promise<UserAccount> { + return (await ax.patch(`/api/user/${userPk}/change_role/`, { role })).data +} + export async function fetchUsers (): Promise<UserAccount[]> { return (await ax.get('api/user/')).data } diff --git a/frontend/src/components/BaseLayout.vue b/frontend/src/components/BaseLayout.vue index 011bd8bf5393f58ec3aa650ac94d8c74d527a6e6..1ac52bd9ddc796abbc3f7ea839aa63fa85a0ba24 100644 --- a/frontend/src/components/BaseLayout.vue +++ b/frontend/src/components/BaseLayout.vue @@ -37,23 +37,6 @@ </v-toolbar> <slot name="sidebar-content" /> <div class="sidebar-footer"> - <v-tooltip - top - style="min-width: 150px" - > - <template #activator="{ on }"> - <div v-on="on"> - <v-switch - v-model="$vuetify.theme.dark" - class="dark-mode-switch" - :disabled="!darkModeUnlocked" - :label="mini ? '' : 'dark mode'" - /> - </div> - </template> - <span v-if="darkModeUnlocked">Experimental: styling issues may occur!</span> - <span v-else>You need to visit the feedback site below first!</span> - </v-tooltip> <v-btn id="feedback-btn" href="https://gitlab.gwdg.de/j.michal/grady/issues" @@ -63,8 +46,7 @@ :text="mini" :tile="!mini" depressed - :class="{ 'fab-button': mini, 'fab-button-white': !darkMode }" - @click.native="logFeedbackClick" + :class="{ 'fab-button': mini }" > <v-icon :left="!mini"> feedback @@ -112,10 +94,6 @@ export default { ...mapStateToComputedGetterSetter({ pathPrefix: 'UI', items: [ - { - name: 'darkModeUnlocked', - mutation: UI.SET_DARK_MODE_UNLOCKED - }, { name: 'mini', path: 'sideBarCollapsed', @@ -124,11 +102,6 @@ export default { ] }) }, - methods: { - logFeedbackClick () { - this.darkModeUnlocked = true - } - } } </script> @@ -154,10 +127,6 @@ export default { .fab-button-white { color: grey !important; } - - .dark-mode-switch { - margin-left: 22px; - } </style> <style> diff --git a/frontend/src/components/TwoPaneLayout.vue b/frontend/src/components/TwoPaneLayout.vue new file mode 100644 index 0000000000000000000000000000000000000000..7d1da6fa439013d651651f4cecdd27520b258db4 --- /dev/null +++ b/frontend/src/components/TwoPaneLayout.vue @@ -0,0 +1,40 @@ +<template> + <v-row + class="pane-wrapper" + no-gutters + > + <v-col class="pane"> + <slot name="left" /> + </v-col> + <v-divider + v-if="showRightPane" + vertical + /> + <v-col + v-if="showRightPane" + class="pane" + > + <slot name="right" /> + </v-col> + </v-row> +</template> + +<script lang="ts"> +import { Vue, Component, Prop } from 'vue-property-decorator' + +@Component +export default class TwoPaneLayout extends Vue { + @Prop({ type: Boolean, default: true }) showRightPane!: boolean +} +</script> + +<style scoped> + .pane-wrapper { + height: 100%; + } + + .pane { + height: 100%; + overflow: auto; + } +</style> diff --git a/frontend/src/components/UserOptions.vue b/frontend/src/components/UserOptions.vue index 7b61d0f7c431b25b4e2f27c7d4c1e3fbc9d3a551..5fffef875f66fef0def2897162eaf4306f8543b1 100644 --- a/frontend/src/components/UserOptions.vue +++ b/frontend/src/components/UserOptions.vue @@ -24,6 +24,16 @@ > Change password </v-list-item> + <v-list-item @click.capture.stop="$vuetify.theme.dark = !$vuetify.theme.dark"> + <v-list-item-content> + <v-list-item-title> + Dark mode (experimental) + </v-list-item-title> + </v-list-item-content> + <v-list-item-action> + <v-switch v-model="$vuetify.theme.dark" /> + </v-list-item-action> + </v-list-item> <v-divider class="my-2" /> <v-list-item @click="logout"> <v-icon left> diff --git a/frontend/src/components/feedback_labels/LabelSelector.vue b/frontend/src/components/feedback_labels/LabelSelector.vue index 02c71161da20069469f73f22d9549e0b8a97b3aa..ee88c72a0ce6dc6d2741dce543069b53c7bc241e 100644 --- a/frontend/src/components/feedback_labels/LabelSelector.vue +++ b/frontend/src/components/feedback_labels/LabelSelector.vue @@ -2,35 +2,28 @@ <v-card> <v-card-title>Assign labels</v-card-title> <v-divider /> - <v-row> - <v-col - class="ml-2" - sm="10" + <v-card-text> + <v-autocomplete + id="label-add-autocomplete" + :items="feedbackLabels" + item-text="name" + item-value="pk" + append-icon="search" + placeholder="search for keywords" + @keyup.enter.ctrl.exact="submitFeedback" + @input="addLabel" > - <v-autocomplete - id="label-add-autocomplete" - :items="feedbackLabels" - item-text="name" - item-value="pk" - append-icon="search" - placeholder="search for keywords" - @keyup.enter.ctrl.exact="submitFeedback" - @input="addLabel" - > - <template #item="{ item }"> - <div class="label-adder-item"> - <feedback-label v-bind="item" /> - </div> - </template> - </v-autocomplete> - </v-col> - <v-row - class="ml-2 mb-3" - > - <v-col sm="4"> - <v-col sm="12"> + <template #item="{ item }"> + <div class="label-adder-item"> + <feedback-label v-bind="item" /> + </div> + </template> + </v-autocomplete> + <v-row> + <v-col md="4"> + <div> CURRENT LABELS - </v-col> + </div> <feedback-label v-for="label in unchangedMapped" :key="label.pk" @@ -39,10 +32,10 @@ @remove-clicked="removeLabel" /> </v-col> - <v-col sm="4"> - <v-col sm="12"> + <v-col md="4"> + <div> WILL BE REMOVED - </v-col> + </div> <feedback-label v-for="label in removedMapped" :key="label.pk" @@ -51,10 +44,10 @@ @remove-clicked="addLabel" /> </v-col> - <v-col sm="4"> - <v-col sm="12"> + <v-col md="4"> + <div> WILL BE ADDED - </v-col> + </div> <feedback-label v-for="label in addedMapped" :key="label.pk" @@ -64,7 +57,7 @@ /> </v-col> </v-row> - </v-row> + </v-card-text> </v-card> </template> diff --git a/frontend/src/components/feedback_list/FeedbackTable.vue b/frontend/src/components/feedback_list/FeedbackTable.vue index 3b6e072423aa520890cb8aa5072ca83deddef404..5a5a491e21a22ab719c59aa0f08e0c4b4d55ff41 100644 --- a/frontend/src/components/feedback_list/FeedbackTable.vue +++ b/frontend/src/components/feedback_list/FeedbackTable.vue @@ -131,6 +131,7 @@ export default class FeedbackTable extends Vue { queryFoundInFields(f: Feedback): boolean { return f.ofSubmissionType !== undefined && this.queryFoundInString(f.ofSubmissionType) || + this.exerciseMode && f.ofStudent !== undefined && this.queryFoundInString(f.ofStudent) || f.created !== undefined && this.queryFoundInString(f.created) || f.modified !== undefined && this.queryFoundInString(f.modified) } diff --git a/frontend/src/components/tutor_list/RoleSelect.vue b/frontend/src/components/tutor_list/RoleSelect.vue new file mode 100644 index 0000000000000000000000000000000000000000..e5236334397af5a92d0f31d5d12678d35ff954f5 --- /dev/null +++ b/frontend/src/components/tutor_list/RoleSelect.vue @@ -0,0 +1,48 @@ +<template> + <v-select + v-model="value" + :items="roleOptions" + filled + dense + hide-details + :loading="loading" + :disabled="isForSelf" + @change="updateRole" + /> +</template> + +<script lang="ts"> +import Vue from 'vue' +import Component from 'vue-class-component' +import { Prop, Watch } from 'vue-property-decorator' +import { Tutor, UserAccount } from '@/models' +import { changeUserRole } from '@/api' +import { Authentication } from '@/store/modules/authentication' + +@Component +export default class RoleSelect extends Vue { + @Prop({ type: Object, required: true }) readonly tutor!: Tutor + + roleOptions = [UserAccount.RoleEnum.Reviewer, UserAccount.RoleEnum.Tutor] + value = this.tutor.role + previousValue = this.value + loading = false + + get isForSelf() { + return Authentication.state.user.pk === this.tutor.pk + } + + async updateRole(newRole: UserAccount.RoleEnum) { + this.loading = true + try { + await changeUserRole(this.tutor.pk, newRole) + this.previousValue = newRole + } catch (error) { + this.value = this.previousValue + } finally { + this.loading = false + } + } +} +</script> + diff --git a/frontend/src/components/tutor_list/TutorList.vue b/frontend/src/components/tutor_list/TutorList.vue index ad203a69a9ae152119fbe8c38df5403036a338dc..e28404b92fda6f636fec0f46cefe16db5c7c7366 100644 --- a/frontend/src/components/tutor_list/TutorList.vue +++ b/frontend/src/components/tutor_list/TutorList.vue @@ -79,6 +79,9 @@ </v-tooltip> </v-btn> </template> + <template #item.role="{ item }"> + <role-select :tutor="item" /> + </template> </v-data-table> </v-card> </template> @@ -92,9 +95,10 @@ import { Authentication } from '@/store/modules/authentication' import { TutorOverview } from '@/store/modules/tutor-overview' import { Group, Tutor } from '@/models' import { Assignments } from '@/store/modules/assignments' +import { Tutor, UserAccount } from '@/models' +import RoleSelect from './RoleSelect.vue' - -@Component +@Component({ components: { RoleSelect } }) export default class TutorList extends Vue { headers = [ { @@ -126,6 +130,10 @@ export default class TutorList extends Vue { text: 'Has Access', align: 'right', value: 'isActive' + }, + { + text: 'Role', + value: 'role' } ] diff --git a/frontend/src/models.ts b/frontend/src/models.ts index 7215c276f41f9a7641e0daf9889e7214b2c3b69e..6e29640d2eeb148efbc89a79869040f47375d7ad 100644 --- a/frontend/src/models.ts +++ b/frontend/src/models.ts @@ -789,6 +789,12 @@ export interface Tutor { * @memberof Tutor */ exerciseGroups: Group[] + + /** + * @type {string} + * @memberof Tutor + */ + role: UserAccount.RoleEnum } /** diff --git a/frontend/src/pages/LayoutSelector.vue b/frontend/src/pages/LayoutSelector.vue index 240a6625f94a167154c7400aec6b4728bd647a5f..bb2e9d04ea1e7ccfc622c18b4647e4a85ef9f839 100644 --- a/frontend/src/pages/LayoutSelector.vue +++ b/frontend/src/pages/LayoutSelector.vue @@ -1,7 +1,7 @@ <template> <div> <component :is="layout" /> - <v-main> + <v-main class="main-content"> <router-view /> </v-main> </div> @@ -39,6 +39,20 @@ export default { } </script> -<style scoped> +<style> + html { + /* Vuetify always shows the scrollbar by default. This disables it. */ + overflow-y: auto !important; + } +</style> +<style scoped> + /* Move the scrollbar below the header so it doesn't jump around when no scrollbar is shown. */ + .main-content { + /* 48px is the vuetify header size. */ + height: calc(100vh - 48px); + margin-top: 48px; + padding-top: 0 !important; + overflow: auto; + } </style> diff --git a/frontend/src/pages/SubscriptionWorkPage.vue b/frontend/src/pages/SubscriptionWorkPage.vue index 5dda389f31c3451463726a3f8a9fd67dc3a5aa0d..a285f24e5763057b771834f1fbee9cc1a8b2450d 100644 --- a/frontend/src/pages/SubscriptionWorkPage.vue +++ b/frontend/src/pages/SubscriptionWorkPage.vue @@ -1,10 +1,8 @@ <template> - <v-container> - <v-row> - <route-change-confirmation :next-route="nextRoute" /> - <v-col - :cols="showSubmissionType ? 6 : 12" - > + <two-pane-layout :show-right-pane="showSubmissionType"> + <template #left> + <v-container> + <route-change-confirmation :next-route="nextRoute" /> <submission-correction :key="currentAssignment.pk" :assignment="currentAssignment" @@ -14,27 +12,20 @@ :tests="submission.tests" :expand="true" /> - </v-col> - - <v-col - v-if="showSubmissionType" - cols="6" - > - <div class="sub-correction"> - <submission-type - :key="submissionType.pk" - v-bind="submissionType" - :reverse="true" - :expanded-by-default="{ Description: true, Solution: true }" - /> - </div> - </v-col> - </v-row> - </v-container> + </v-container> + </template> + <template #right> + <submission-type + :key="submissionType.pk" + v-bind="submissionType" + :expanded-by-default="{ Description: true, Solution: true }" + /> + </template> + </two-pane-layout> </template> <script lang="ts"> -import { Vue, Component} from 'vue-property-decorator' +import { Vue, Component, Watch } from 'vue-property-decorator' import { Route, NavigationGuard } from 'vue-router' import SubmissionCorrection from '@/components/submission_notes/SubmissionCorrection.vue' import SubmissionType from '@/components/submission_type/SubmissionType.vue' @@ -46,6 +37,7 @@ import RouteChangeConfirmation from '@/components/submission_notes/RouteChangeCo import { getters } from '@/store/getters' import { SubmissionAssignment } from '@/models' import { UI } from '@/store/modules/ui' +import TwoPaneLayout from '@/components/TwoPaneLayout.vue' const onRouteEnterOrUpdate: NavigationGuard = function (to, from, next) { Assignments.changeAssignment(to).then(() => { @@ -61,7 +53,8 @@ const onRouteEnterOrUpdate: NavigationGuard = function (to, from, next) { RouteChangeConfirmation, SubmissionTests, SubmissionType, - SubmissionCorrection + SubmissionCorrection, + TwoPaneLayout, } }) export default class SubscriptionWorkPage extends Vue { @@ -116,14 +109,10 @@ export default class SubscriptionWorkPage extends Vue { this.$router.replace({name: 'correction-ended'}) }) } -} -</script> -<style scoped> - .sub-correction { - position: sticky; - top: 64px; - overflow-y: auto; - height: 90vh; + @Watch('currentAssignment') + onCurrentAssignmentChanged() { + window.scrollTo(0, 0) } -</style> +} +</script> diff --git a/frontend/src/pages/base/FeedbackHistoryPage.vue b/frontend/src/pages/base/FeedbackHistoryPage.vue index 1d5e4570ee8be3473b8d344a67e11e598549175d..313494d96e98c64c45708ec43324d53521713119 100644 --- a/frontend/src/pages/base/FeedbackHistoryPage.vue +++ b/frontend/src/pages/base/FeedbackHistoryPage.vue @@ -1,29 +1,28 @@ <template> - <v-container> - <v-row> - <v-col cols="6"> - <router-view name="left" /> - </v-col> - <v-col - cols="6" - > - <div class="right-view"> - <router-view - name="right" - @refresh="refresh" - /> - </div> - </v-col> - </v-row> - </v-container> + <two-pane-layout> + <template #left> + <router-view name="left" /> + </template> + <template #right> + <v-container> + <router-view + name="right" + @refresh="refresh" + /> + </v-container> + </template> + </two-pane-layout> </template> <script lang="ts"> import Vue from 'vue' import Component from 'vue-class-component' import { FeedbackTable } from '@/store/modules/feedback_list/feedback-table' +import TwoPaneLayout from '@/components/TwoPaneLayout.vue' -@Component +@Component({ + components: { TwoPaneLayout } +}) export default class FeedbackHistoryPage extends Vue { refresh () { FeedbackTable.getFeedbackHistory() @@ -34,12 +33,3 @@ export default class FeedbackHistoryPage extends Vue { } } </script> - -<style scoped> - .right-view { - position: sticky; - top: 80px; - overflow-y: scroll; - height: 90vh; - } -</style> diff --git a/frontend/src/store/modules/ui.ts b/frontend/src/store/modules/ui.ts index c200c0e8dab04dc143550976578086d53dd55d71..7e3849be84ff6d04033ff6aa84f45e1c05a8f67b 100644 --- a/frontend/src/store/modules/ui.ts +++ b/frontend/src/store/modules/ui.ts @@ -3,7 +3,6 @@ import { RootState } from '@/store/store' export interface UIState { sideBarCollapsed: boolean - darkModeUnlocked: boolean showJumbotron: boolean, showSubmissionType: boolean, } @@ -11,7 +10,6 @@ export interface UIState { function initialState (): UIState { return { sideBarCollapsed: false, - darkModeUnlocked: false, showJumbotron: true, showSubmissionType: true, } @@ -24,9 +22,6 @@ const stateGetter = mb.state() function SET_SIDEBAR_COLLAPSED (state: UIState, collapsed: boolean) { state.sideBarCollapsed = collapsed } -function SET_DARK_MODE_UNLOCKED (state: UIState, val: boolean) { - state.darkModeUnlocked = val -} function SET_SHOW_JUMBOTRON (state: UIState, val: boolean) { state.showJumbotron = val } @@ -38,7 +33,6 @@ export const UI = { get state () { return stateGetter() }, SET_SIDEBAR_COLLAPSED: mb.commit(SET_SIDEBAR_COLLAPSED), - SET_DARK_MODE_UNLOCKED: mb.commit(SET_DARK_MODE_UNLOCKED), SET_SHOW_JUMBOTRON: mb.commit(SET_SHOW_JUMBOTRON), SET_SHOW_SUBMISSIONTYPE: mb.commit(SET_SHOW_SUBMISSIONTYPE), }