Merge pull request #20 from leaonline/feature-ensure-grade-name

Feedback grading keeps original gradeName

.github/workflows/test_suite.yml

@@ -164,7 +164,7 @@ jobs:
         run: meteor npm ci
 
       - name: Run Tests
-        run: sh ./test.sh -c
+        run: sh ./test.sh -c -o # run -o once with -c coverage
 
       - name: Upload coverage
         uses: actions/upload-artifact@v2

imports/api/accounts/tests/generateUserCode.tests.js

@@ -4,6 +4,7 @@ import { generateUserCode } from '../generateUserCode'
 
 describe(generateUserCode.name, function () {
   it('generates a random code of given length', function () {
+    this.timeout(5000)
     for (let i = 0; i < 1000; i++) {
       expect(generateUserCode().length).to.equal(5)
     }

imports/api/context/initClientContext.js

@@ -2,7 +2,7 @@ import { createCollection } from '../../infrastructure/factories/collection/crea
 import Collection2 from 'meteor/aldeed:collection2'
 
 const created = new Set()
-const collection2Init = false
+let collection2Init = false
 
 /**
  * Lightweight initialization for contexts on the client-side.
@@ -17,7 +17,11 @@ export const initClientContext = (context, debug = console.debug) => {
   }
 
   if (!collection2Init) {
-    Collection2.load()
+    // XXX: backwards compat for pre 4.0 collection2
+    if (Collection2 && typeof Collection2.load === 'function') {
+      Collection2.load()
+    }
+    collection2Init = true
   }
 
   createCollection(context, debug)

imports/api/context/tests/initClientContext.tests.js

@@ -12,19 +12,24 @@ describe(initClientContext.name, function () {
   afterEach(function () {
     restoreAll()
   })
-  it('loads collection2 lazy', function () {
-    let loadCalled = false
-    stub(Collection2, 'load', function () {
-      loadCalled = true
-    })
 
-    initClientContext({
-      name: Random.id(),
-      schema: { title: String }
-    }, debug)
+  // XXX: backwards compat for pre 4.0 collection2
+  if (Collection2 && typeof Collection2.load === 'function') {
+    it('loads collection2 lazy', function () {
+      let loadCalled = false
+      stub(Collection2, 'load', function () {
+        loadCalled = true
+      })
+
+      initClientContext({
+        name: Random.id(),
+        schema: { title: String }
+      }, debug)
+
+      expect(loadCalled).to.equal(true)
+    })
+  }
 
-    expect(loadCalled).to.equal(true)
-  })
   it('creates a new collection', function () {
     const ctx = {
       name: Random.id(),

imports/api/http/fetchDoc.js

@@ -2,13 +2,15 @@ import { Meteor } from 'meteor/meteor'
 import { HTTP } from 'meteor/jkuester:http'
 
 const origin = Meteor.absoluteUrl()
-const headers = {
+const defaultHeaders = {
   origin: origin,
   mode: 'cors',
   cache: 'no-store'
 }
 
 export const fetchDoc = (url, params) => {
+  const headers = { ...defaultHeaders }
+  console.debug('[fetchDoc]:', { url, headers, params })
   const requestOptions = { params, headers }
   const response = HTTP.get(url, requestOptions)
   return response.data

imports/contexts/feedback/Feedback.js

@@ -1,3 +1,9 @@
+/**
+ * A feedback represents a evaluation-snapshot of one TestCycle, independent
+ * from others or recent ones.
+ *
+ * This is to give immediate feedback in regards to this TestCycle to the user.
+ */
 export const Feedback = {
   name: 'feedback',
   label: 'feedback.title',

imports/contexts/feedback/api/generateFeedback.js

@@ -8,6 +8,41 @@ import { getAlphaLevels } from './getAlphaLevels'
 import { getCompetencies } from './getCompetencies'
 import { notifyUsersAboutError } from '../../../api/notify/notifyUsersAboutError'
 
+/**
+ * The general evaluation algorithm for a single TestCycle.
+ * This can be considered "atomic" to the extend, that it won't look
+ * into prior TestCycles to generate comparative results.
+ * It works by collecting the responses, competencies and their alpha levels,
+ * which were covered during a TestCycle.
+ *
+ * It then compares the scores for each competency against thresholds (defined
+ * in the lea. backend) and assigns a gradeName (how the score is titled),
+ * a gradeIndex (the position in the thresholds list) and a isGraded flag
+ * (indicating, that this grade may not be accurate due to not enough covered
+ * items during the TestCycle).
+ *
+ * It then creates a mean for all competencies, that associate with the same
+ * alpha level and uses this mean score in order to apply the same grading
+ * mechanism (as described above) but with alpha level.
+ *
+ * The advantage of this approach is, that we have a clear and reproducible
+ * procedure, easily testable with few parameters (compared to other models).
+ *
+ * The drawback of this approach is, that we might face a situation, where
+ * we never have enough competencies covered to reach a certain threshold
+ * and since we don't compare with recent feedback, we may end up having
+ * many competencies marked as isGraded = false
+ *
+ * Note: isGraded means here "reached the threshold of minimum count of graded
+ * occurrences" - it just remains isGraded due to backwards compatibility.
+ *
+ * @param options
+ * @param options.sessionDoc {document} the related doc from this session
+ * @param options.testCycleDoc {document} the related doc from this session's testCycle
+ * @param options.userId {string} the user, for which this feedback applies
+ * @param options.debug {function=} optional function for debugging
+ * @return {*}
+ */
 export const generateFeedback = (options) => {
   check(options, {
     sessionDoc: Match.ObjectIncluding({ _id: String }),
@@ -237,7 +272,7 @@ export const gradeCompetenciesAndCountAlphaLevels = ({ competencies, minCountAlp
 
     current.gradeName = grade.name
     current.gradeIndex = grade.index
-    current.isGraded = grade.index > -1
+    current.isGraded = !grade.notEnough
 
     competencies.set(competencyId, current)
 
@@ -295,7 +330,7 @@ export const gradeAlphaLevels = ({ alphaLevels, thresholds }) => {
 
     alpha.gradeName = grade.name
     alpha.gradeIndex = grade.index
-    alpha.isGraded = grade.index > -1
+    alpha.isGraded = !grade.notEnough
 
     alphaLevels.set(alphaLevelId, alpha)
   })

imports/contexts/feedback/tests/generateFeedback.tests.js

@@ -160,8 +160,10 @@ describe(gradeCompetenciesAndCountAlphaLevels.name, function () {
       undef: 0,
       min: 3,
       perc: 0.5,
-      gradeName: 'notEnough',
-      gradeIndex: -1,
+      gradeName: 'ok',
+      gradeIndex: 2,
+      // note how isGraded: false does not affect
+      // hypotehtical gradeName and gradeIndex
       isGraded: false
     })
   })
@@ -387,8 +389,10 @@ describe(gradeAlphaLevels.name, function () {
       count: 2,
       scored: 2,
       perc: 1,
-      gradeName: 'notEnough',
-      gradeIndex: -1,
+      gradeName: 'top',
+      gradeIndex: 0,
+      // note how isGraded:false does not affect
+      // the hypothetical gradeName and gradeIndex
       isGraded: false
     })
   })
@@ -717,8 +721,10 @@ describe(generateFeedback.name, function () {
       }, {
         competencyId: cid3,
         count: 1,
-        gradeIndex: -1,
-        gradeName: 'notEnough',
+        gradeIndex: 0,
+        gradeName: 'good',
+        // note how isGraded: false does not affect
+        // hypothetical gradeName and gradeIndex
         isGraded: false,
         perc: 1,
         scored: 1,
@@ -798,8 +804,8 @@ describe(generateFeedback.name, function () {
       }, {
         competencyId: cid3,
         count: 1,
-        gradeIndex: -1,
-        gradeName: 'notEnough',
+        gradeIndex: 0,
+        gradeName: 'good',
         isGraded: false,
         perc: 1,
         scored: 1,
@@ -878,8 +884,8 @@ describe(generateFeedback.name, function () {
       }, {
         competencyId: cid3,
         count: 1,
-        gradeIndex: -1,
-        gradeName: 'notEnough',
+        gradeIndex: 0,
+        gradeName: 'good',
         isGraded: false,
         perc: 1,
         scored: 1,

imports/contexts/record/Record.js

@@ -1,15 +1,15 @@
 import { onServerExec } from '../../utils/archUtils'
 
 /**
- * The record is a dataset for each user, that represents the current state,
- * like a snapshot to be used for evaluation purposes (as with the teacher's
- * dashboard application).
+ * The record is a dataset for each user for evaluation purposes
+ * (as with the teacher's dashboard application).
  *
- * It summarizes, which tests (sessions) were made and which results were
- * assigned and which documents are associated to minimize effort for loading
+ * It summarizes, which TestCycles (sessions) were completed and which results
+ * were assigned and which documents are associated to minimize effort for loading
  * and processing such data.
  *
- * A record entry is unique by
+ * In contrast to Feedback it also contains comparable information between the
+ * records. See Record.status for more information.
  */
 export const Record = {
   name: 'record',
@@ -19,21 +19,25 @@ export const Record = {
   publications: {}
 }
 
+/**
+ * The record contains info on how this recent feedback compares to past ones.
+ * It always compares to the immediate predecessor.
+ */
 Record.status = {
   /**
-   * There is no previous record
+   * There is no previous record for the same TestCycle
    */
   new: 'new',
   /**
    * Perc values of the previous and current record are the same
    */
   same: 'same',
   /**
-   * Current is "better" than previous
+   * Current is "better" than previous; perc value is higher
    */
   improved: 'improved',
   /**
-   * Current is "worse" tha previous
+   * Current is "worse" tha previous; perc value is lower
    */
   declined: 'declined'
 }
@@ -48,6 +52,10 @@ Record.schema = {
   testCycle: String,
   session: String,
   feedback: String,
+  previousId: {
+    type: String, // the previous record doc id to be compared with
+    optional: true
+  },
 
   // timestamps
   startedAt: Date,
@@ -116,20 +124,10 @@ Record.methods.getForUsers = {
   },
   backend: true,
   run: onServerExec(function () {
-    import { Meteor } from 'meteor/meteor'
-
-    const { defaultOldest } = Meteor.settings.records
     const transform = {
       hint: { $natural: -1 }
     }
 
-    const getMaxOldest = () => {
-      const maxOldest = new Date()
-      const aMonthAgo = maxOldest.getDate() - defaultOldest
-      maxOldest.setDate(aMonthAgo)
-      return maxOldest
-    }
-
     return function ({ users = [], dimension, skip = [], oldest, newest }) {
       const query = {
         userId: { $in: users },
@@ -143,23 +141,17 @@ Record.methods.getForUsers = {
         query._id = { $nin: skip }
       }
 
-      const maxOldest = getMaxOldest()
-
       // we can let users determine an "oldest"
       // as long as it's not older than the oldest possible
 
-      if (oldest && oldest > maxOldest) {
+      if (oldest) {
         query.completedAt = { $gte: oldest }
       }
 
-      else {
-        query.completedAt = { $gte: maxOldest }
-      }
-
       // we can also let users determine oldest date
       // as long as it's not older than the oldest possible
 
-      if (newest && newest > maxOldest) {
+      if (newest) {
         query.completedAt = { $lte: newest }
       }
 

imports/contexts/record/api/addRecord.js

@@ -9,6 +9,13 @@ const byCompetencyId = c => c.competencyId
 const byAlphaLevelId = a => a.alphaLevelId
 
 /**
+ * Creates a new record document for a given feedback and testCycle.
+ *
+ * It looks first, whether another record has been created today. This is,
+ * because records are focused on a single date. If a testCycle has been
+ * completed multiple times on the same date, then it will only update
+ * the doc. Otherwise we cannot create a comparable overview.
+ *
  *
  * @param options
  * @param options.userId {string}
@@ -35,6 +42,7 @@ export const addRecord = (options) => {
       alphaLevels: [Object]
     })
   }))
+
   const { userId, testCycleDoc, sessionDoc, feedbackDoc } = options
   const { dimension, level } = testCycleDoc
   const { startedAt, completedAt, cancelledAt, continuedAt } = sessionDoc
@@ -46,6 +54,8 @@ export const addRecord = (options) => {
   compareDate.setSeconds(0)
   compareDate.setMilliseconds(0)
 
+  // TODO: update the current record if we already have a record doc generated
+
   const previousRecordData = getPreviousRecord({
     userId: userId,
     dimension: dimension,
@@ -68,9 +78,10 @@ export const addRecord = (options) => {
     const previousDoc = previousRecordData.competency(competencyId)
 
     let status = Record.status.new
-
+    let previousId = ''
     if (previousDoc) {
       status = getStatus(previousDoc.perc, feedbackCompetency.perc)
+      previousId = previousDoc._id
     }
 
     return {
@@ -86,6 +97,7 @@ export const addRecord = (options) => {
       isGraded: feedbackCompetency.isGraded,
       gradeName: feedbackCompetency.gradeName,
       example: feedbackCompetency.example,
+      previousId: previousId,
       development: status
     }
   })