From 35ab9913130a0402aaa5f365af45f4761d36c3f6 Mon Sep 17 00:00:00 2001
From: "robinwilliam.hundt" <robinwilliam.hundt@stud.uni-goettingen.de>
Date: Sat, 29 Sep 2018 14:14:13 +0200
Subject: [PATCH] fixed #124

also removed one passible cause for memory leak. Feedback was unnecesarily stored in the store under state.feedback
---
 frontend/src/components/BaseLayout.vue        |  5 +--
 .../submission_notes/SubmissionCorrection.vue |  5 ++-
 .../src/pages/StudentSubmissionSideView.vue   |  6 ++--
 .../src/pages/base/FeedbackHistoryPage.vue    | 16 ++++++---
 frontend/src/store/actions.ts                 | 33 +++++++++----------
 frontend/src/store/modules/subscriptions.ts   | 22 ++++++++-----
 frontend/src/store/mutations.ts               | 13 --------
 frontend/src/store/store.ts                   |  2 --
 frontend/vue.config.js                        |  5 +++
 9 files changed, 54 insertions(+), 53 deletions(-)

diff --git a/frontend/src/components/BaseLayout.vue b/frontend/src/components/BaseLayout.vue
index db4e169c..4c98bb6d 100644
--- a/frontend/src/components/BaseLayout.vue
+++ b/frontend/src/components/BaseLayout.vue
@@ -77,7 +77,7 @@
           <user-options/>
         </v-menu>
       </div>
-      <v-btn color="blue darken-1" to="/" @click.native="logout">Logout</v-btn>
+      <v-btn color="blue darken-1" @click.native="logout">Logout</v-btn>
       <slot name="toolbar-right"></slot>
     </v-toolbar>
   </div>
@@ -89,6 +89,7 @@ import {UI} from '@/store/modules/ui'
 import { mapStateToComputedGetterSetter } from '@/util/helpers'
 import UserOptions from '@/components/UserOptions'
 import {Authentication} from '@/store/modules/authentication'
+import { actions } from '@/store/actions';
 
 export default {
   name: 'base-layout',
@@ -124,7 +125,7 @@ export default {
   },
   methods: {
     logout () {
-      this.$store.dispatch('logout')
+      actions.logout()
     },
     logFeedbackClick () {
       this.darkModeUnlocked = true
diff --git a/frontend/src/components/submission_notes/SubmissionCorrection.vue b/frontend/src/components/submission_notes/SubmissionCorrection.vue
index 20378cee..cd802011 100644
--- a/frontend/src/components/submission_notes/SubmissionCorrection.vue
+++ b/frontend/src/components/submission_notes/SubmissionCorrection.vue
@@ -64,13 +64,12 @@ import AnnotatedSubmissionBottomToolbar from '@/components/submission_notes/tool
 import BaseAnnotatedSubmission from '@/components/submission_notes/base/BaseAnnotatedSubmission'
 import SubmissionLine from '@/components/submission_notes/base/SubmissionLine'
 import {SubmissionNotes} from '@/store/modules/submission-notes'
-import RouteChangeConfirmation from '@/components/submission_notes/RouteChangeConfirmation'
 import {Authentication} from '@/store/modules/authentication'
 import { actions } from '@/store/actions'
+import { fetchFeedback } from '@/api';
 
 export default {
   components: {
-    RouteChangeConfirmation,
     SubmissionLine,
     BaseAnnotatedSubmission,
     AnnotatedSubmissionBottomToolbar,
@@ -144,7 +143,7 @@ export default {
     shortPollOrigFeedback () {
       this.feedbackShortPollInterval = setInterval(() => {
         if (this.feedbackObj && this.feedbackObj.ofSubmission) {
-          actions.getFeedback({ofSubmission: this.feedbackObj.ofSubmission}).then(feedback => {
+          fetchFeedback({ofSubmission: this.feedbackObj.ofSubmission}).then(feedback => {
             SubmissionNotes.SET_ORIG_FEEDBACK(feedback)
           })
         }
diff --git a/frontend/src/pages/StudentSubmissionSideView.vue b/frontend/src/pages/StudentSubmissionSideView.vue
index 275844ae..868d92ef 100644
--- a/frontend/src/pages/StudentSubmissionSideView.vue
+++ b/frontend/src/pages/StudentSubmissionSideView.vue
@@ -27,6 +27,7 @@ import SubmissionCorrection from '@/components/submission_notes/SubmissionCorrec
 import SubmissionTests from '@/components/SubmissionTests'
 import SubmissionType from '@/components/SubmissionType'
 import RouteChangeConfirmation from '@/components/submission_notes/RouteChangeConfirmation'
+import { actions } from '@/store/actions';
 
 function onRouteEnterOrUpdate (to, from, next) {
   const toIsSubmissionSideView = to.matched.some(route => route.meta.submissionSideView)
@@ -78,12 +79,13 @@ export default {
   },
   methods: {
     refresh () {
+      this.$emit('refresh')
       const studentPk = this.$route.params.studentPk
       if (studentPk) {
-        this.$store.dispatch('getStudents', {studentPks: [studentPk]})
+        actions.getStudents({studentPks: [studentPk]})
       }
       const submissionPk = this.$route.params.submissionPk
-      this.$store.dispatch('getSubmissionFeedbackTest', {pk: submissionPk})
+      actions.getSubmissionFeedbackTest({pk: submissionPk})
     }
   },
   beforeRouteEnter (to, from, next) {
diff --git a/frontend/src/pages/base/FeedbackHistoryPage.vue b/frontend/src/pages/base/FeedbackHistoryPage.vue
index 4e0d8b61..d93be5fc 100644
--- a/frontend/src/pages/base/FeedbackHistoryPage.vue
+++ b/frontend/src/pages/base/FeedbackHistoryPage.vue
@@ -5,19 +5,27 @@
     </v-flex>
     <v-flex xs6 class="ml-3">
       <div class="right-view">
-        <router-view name="right"></router-view>
+        <router-view name="right" @refresh="refresh"></router-view>
       </div>
     </v-flex>
   </v-layout>
 </template>
 
-<script>
+<script lang="ts">
+import {Vue, Component} from 'vue-property-decorator'
 import { FeedbackTable } from '@/store/modules/feedback_list/feedback-table'
-export default {
-  name: 'feedback-history-page',
+
+@Component
+export default class FeedbackHistoryPage extends Vue {
+
+  refresh () {
+    FeedbackTable.getFeedbackHistory()
+  }
+
   created () {
     FeedbackTable.getFeedbackHistory()
   }
+
 }
 </script>
 
diff --git a/frontend/src/store/actions.ts b/frontend/src/store/actions.ts
index 3d5982c3..41324038 100644
--- a/frontend/src/store/actions.ts
+++ b/frontend/src/store/actions.ts
@@ -46,14 +46,6 @@ async function getTutors () {
   const tutors = await api.fetchAllTutors()
   mut.SET_TUTORS(tutors)
 }
-async function getFeedback (
-  context: BareActionContext<RootState, RootState>,
-  fetchFeedbackArg: { ofSubmission: string }
-) {
-  const feedback = await api.fetchFeedback(fetchFeedbackArg)
-  mut.SET_FEEDBACK(feedback)
-  return feedback
-}
 async function getSubmissionFeedbackTest (
   context: BareActionContext<RootState, RootState>,
   submissionPkObj: { pk: string }
@@ -65,6 +57,16 @@ async function getStatistics () {
   const statistics = await api.fetchStatistics()
   mut.SET_STATISTICS(statistics)
 }
+
+function resetState ({message}: {message: string}) {
+  FeedbackTable.RESET_STATE()
+  Subscriptions.RESET_STATE()
+  SubmissionNotes.RESET_STATE()
+  Authentication.RESET_STATE()
+  Authentication.SET_MESSAGE(message)
+  mut.RESET_STATE()
+}
+
 function logout (
   context: BareActionContext<RootState, RootState>,
   message = ''
@@ -74,15 +76,11 @@ function logout (
     // TODO this should belong in auth module
     api.changeActiveForUser(Authentication.state.user.pk, false)
   }
-  router.push({ name: 'login' })
-  // there should be a better way to solve the issue of resetting the once() function used in subscription module
-  mut.RESET_STATE()
-  FeedbackTable.RESET_STATE()
-  Authentication.RESET_STATE()
-  Subscriptions.RESET_STATE()
-  SubmissionNotes.RESET_STATE()
-  Authentication.SET_MESSAGE(message)
-  router.go(0)
+  router.push({ name: 'login' }, () => {
+    resetState({message})
+    // there should be a better way to solve the issue of resetting the once() function used in subscription module
+    router.go(0)
+  })
 }
 
 const mb = getStoreBuilder<RootState>()
@@ -92,7 +90,6 @@ export const actions = {
   getExamTypes: mb.dispatch(getExamTypes),
   getStudents: mb.dispatch(getStudents),
   getTutors: mb.dispatch(getTutors),
-  getFeedback: mb.dispatch(getFeedback),
   getSubmissionFeedbackTest: mb.dispatch(getSubmissionFeedbackTest),
   getStatistics: mb.dispatch(getStatistics),
   logout: mb.dispatch(logout)
diff --git a/frontend/src/store/modules/subscriptions.ts b/frontend/src/store/modules/subscriptions.ts
index 3fbfe78a..7c745d74 100644
--- a/frontend/src/store/modules/subscriptions.ts
+++ b/frontend/src/store/modules/subscriptions.ts
@@ -76,23 +76,27 @@ const resolveSubscriptionKeyToNameGetter = mb.read(function resolveSubscriptionK
     }
   }
 })
+
+type SubscriptionsByStage = {[p in Subscription.FeedbackStageEnum]?: {[k in Subscription.QueryTypeEnum]: Subscription[]}}
 // TODO Refactor this monstrosity
 const getSubscriptionsGroupedByTypeGetter = mb.read(function getSubscriptionsGroupedByType (state, getters) {
   const subscriptionsByType = () => {
     return {
-      'random': [],
-      'student': [],
-      'exam': [],
-      'submission_type': []
+      [Subscription.QueryTypeEnum.Random] : [],
+      [Subscription.QueryTypeEnum.Student]: [],
+      [Subscription.QueryTypeEnum.Exam]: [],
+      [Subscription.QueryTypeEnum.SubmissionType]: []
     }
   }
-  let subscriptionsByStage = getters.availableStages.reduce((acc: {[p: string]: {[k: string]: Subscription[]}},
-    curr: string) => {
+  let subscriptionsByStage: SubscriptionsByStage = Subscriptions.availableStages.reduce((acc: SubscriptionsByStage,
+    curr: Subscription.FeedbackStageEnum) => {
     acc[curr] = subscriptionsByType()
     return acc
   }, {})
-  Object.values(state.subscriptions).forEach((subscription: any) => {
-    subscriptionsByStage[subscription.feedbackStage][subscription.queryType].push(subscription)
+  Object.values(state.subscriptions).forEach((subscription: Subscription) => {
+    if (subscriptionsByStage && subscription.feedbackStage && subscription.queryType) {
+      subscriptionsByStage[subscription.feedbackStage]![subscription.queryType].push(subscription)
+    }
   })
   // sort the resulting arrays in subscriptions lexicographically by their query_keys
   const sortSubscriptions = (subscriptionsByType: {[k: string]: Subscription[]}) => Object.values(subscriptionsByType)
@@ -147,7 +151,7 @@ async function subscribeTo (
   {type, key, stage}:
   {type: Subscription.QueryTypeEnum, key?: string, stage: Subscription.FeedbackStageEnum}): Promise<Subscription> {
   // don't subscribe to type, key, stage combinations if they're already present
-  let subscription = Subscriptions.getSubscriptionsGroupedByType[stage][type].find((elem: Subscription) => {
+  let subscription = Subscriptions.getSubscriptionsGroupedByType[stage]![type].find((elem: Subscription) => {
     if (type === Subscription.QueryTypeEnum.Random) {
       return true
     }
diff --git a/frontend/src/store/mutations.ts b/frontend/src/store/mutations.ts
index b4e343be..a75339b7 100644
--- a/frontend/src/store/mutations.ts
+++ b/frontend/src/store/mutations.ts
@@ -40,18 +40,6 @@ function SET_STATISTICS (state: RootState, statistics: Statistics) {
         ...statistics
     }
 }
-function SET_FEEDBACK (state: RootState, feedback: Feedback) {
-    if (!feedback.ofSubmissionType) {
-        throw new Error("Can only SET_FEEDBACK when ofSubmissionType is set")
-    }
-    // weird cast is necessary because of the type of Vue.set
-    Vue.set(state.feedback, <string><any>feedback.pk, {
-        ...state.feedback[feedback.pk],
-        ...feedback,
-        // TODO fix this fucking memory leak
-        ofSubmissionType: state.submissionTypes[feedback.ofSubmissionType]
-    })
-}
 function UPDATE_SUBMISSION_TYPE (state: RootState, submissionType: SubmissionType) {
     const updatedSubmissionType = {
         ...state.submissionTypes[submissionType.pk],
@@ -89,7 +77,6 @@ export const mutations = {
     SET_TUTORS: mb.commit(SET_TUTORS),
     SET_SUBMISSION: mb.commit(SET_SUBMISSION),
     SET_STATISTICS: mb.commit(SET_STATISTICS),
-    SET_FEEDBACK: mb.commit(SET_FEEDBACK),
     UPDATE_SUBMISSION_TYPE: mb.commit(UPDATE_SUBMISSION_TYPE),
     RESET_STATE: mb.commit(RESET_STATE)
 }
diff --git a/frontend/src/store/store.ts b/frontend/src/store/store.ts
index 9863a0d7..b27d7a96 100644
--- a/frontend/src/store/store.ts
+++ b/frontend/src/store/store.ts
@@ -39,7 +39,6 @@ Vue.use(Vuex)
 export interface RootInitialState {
     lastAppInteraction: number
     examTypes: {[pk: string]: Exam}
-    feedback: {[pk: number]: Feedback}
     submissionTypes: {[pk: string]: SubmissionType}
     submissions: {[pk: string]: SubmissionNoType}
     students: {[pk: string]: StudentInfoForListView}
@@ -62,7 +61,6 @@ export function initialState (): RootInitialState {
   return {
     lastAppInteraction: Date.now(),
     examTypes: {},
-    feedback: {},
     submissionTypes: {},
     submissions: {},
     students: {},
diff --git a/frontend/vue.config.js b/frontend/vue.config.js
index d8e6fdce..f470691e 100644
--- a/frontend/vue.config.js
+++ b/frontend/vue.config.js
@@ -10,6 +10,11 @@ module.exports = {
   },
   configureWebpack: config => {
     config.resolve.alias['@'] = `${projectRoot}/src`
+
+    if (process.env.NODE_ENV === 'development') {
+      config.devtool = 'source-map'
+    }
+
     // keep_fnames ist set to true because vuex-typex is dependant on the function names
     if (process.env.NODE_ENV === 'production') {
       config.optimization.minimizer[0].options.uglifyOptions.keep_fnames = true
-- 
GitLab