[#12544] Rubric Question Statistics: Handle empty weights (#12545)

* Use NO_VALUE to represent empty weights * Skip over null weights * Clean up calculations * Standardize naming * Fix null weight display * Add explanation for empty weights * Fix downloaded result empty weight display * Fix order of assertEquals * Handle null conversion to NO_VALUE when input out of focus * Revert back to using null value * Fix display when all chosen weights are empty * Standardize display for empty values * Fix failing tests
TEAMMATES · Aug 19, 2023 · b49359e · b49359e
1 parent 325d066
commit b49359e
Show file tree

Hide file tree

Showing 7 changed files with 144 additions and 59 deletions.
diff --git a/src/e2e/java/teammates/e2e/pageobjects/InstructorFeedbackEditPage.java b/src/e2e/java/teammates/e2e/pageobjects/InstructorFeedbackEditPage.java
@@ -741,8 +741,8 @@ public void verifyRubricQuestionDetails(int questionNum, FeedbackRubricQuestionD
             for (int i = 0; i < numSubQn; i++) {
                 List<WebElement> rubricWeights = getRubricWeights(questionNum, i + 2);
                 for (int j = 0; j < numChoices; j++) {
-                    assertEquals(rubricWeights.get(j).getAttribute("value"),
-                            getDoubleString(weights.get(i).get(j)));
+                    assertEquals(getDoubleString(weights.get(i).get(j)),
+                            rubricWeights.get(j).getAttribute("value"));
                 }
             }
         } else {

diff --git a/...uestion-types/question-edit-details-form/rubric-question-edit-details-form.component.html b/...uestion-types/question-edit-details-form/rubric-question-edit-details-form.component.html
@@ -2,7 +2,7 @@
   <div class="col-12 text-start">
     <div class="form-group form-check">
       <label class="form-check-label tool-tip-decorate ngb-tooltip-class"
-             ngbTooltip="Assign weights to the columns for calculating statistics.">
+             ngbTooltip="Assign weights to the columns for calculating statistics. An empty weight (i.e. Not enough information to evaluate) can be assigned by leaving the input box empty.">
         <input id="weights-checkbox" type="checkbox" class="form-check-input" [disabled]="!isEditable"
                [ngModel]="model.hasAssignedWeights" (ngModelChange)="triggerChoicesWeight($event)">Choices are weighted</label>
     </div>
@@ -49,7 +49,8 @@
                     (ngModelChange)="triggerRubricDescriptionChange($event, i, j)" rows="3" [disabled]="!isEditable"></textarea>
           <input *ngIf="model.hasAssignedWeights" type="number" class="form-control margin-top-10px" step="0.01" aria-label="Choice weight input"
                  [disabled]="!isEditable"
-                 [ngModel]="model.rubricWeightsForEachCell[i][j]" (ngModelChange)="triggerRubricWeightChange($event, i, j)">
+                 [ngModel]="model.rubricWeightsForEachCell[i][j]"
+                 (ngModelChange)="triggerRubricWeightChange($event, i, j)">
         </td>
       </tr>
       <tr>

diff --git a/...tion-statistics/question-statistics-calculation/rubric-question-statistics-calculation.ts b/...tion-statistics/question-statistics-calculation/rubric-question-statistics-calculation.ts
@@ -3,7 +3,7 @@ import {
   FeedbackRubricQuestionDetails,
   FeedbackRubricResponseDetails,
 } from '../../../../../types/api-output';
-import { RUBRIC_ANSWER_NOT_CHOSEN } from '../../../../../types/feedback-response-details';
+import { NO_VALUE, RUBRIC_ANSWER_NOT_CHOSEN } from '../../../../../types/feedback-response-details';
 import { QuestionStatistics } from '../question-statistics';
 
 /**
@@ -18,6 +18,7 @@ export interface PerRecipientStats {
   percentages: number[][];
   percentagesAverage: number[];
   weightsAverage: number[];
+  areSubQuestionChosenWeightsAllNull: boolean[];
   subQuestionTotalChosenWeight: number[];
   subQuestionWeightAverage: number[];
   overallWeightedSum: number;
@@ -113,6 +114,7 @@ export class RubricQuestionStatisticsCalculation
           percentages: [],
           percentagesAverage: [],
           weightsAverage: [],
+          areSubQuestionChosenWeightsAllNull: this.subQuestions.map(() => true),
           subQuestionTotalChosenWeight: this.subQuestions.map(() => 0),
           subQuestionWeightAverage: [],
         };
@@ -122,38 +124,67 @@ export class RubricQuestionStatisticsCalculation
           continue;
         }
         this.perRecipientStatsMap[response.recipientEmail || response.recipient].answers[i][subAnswer] += 1;
-        this.perRecipientStatsMap[response.recipientEmail || response.recipient].subQuestionTotalChosenWeight[i] +=
-            +this.weights[i][subAnswer].toFixed(5);
+        if (this.weights[i][subAnswer] !== null) {
+            this.perRecipientStatsMap[response.recipientEmail || response.recipient].subQuestionTotalChosenWeight[i] +=
+                +this.weights[i][subAnswer].toFixed(5);
+            this.perRecipientStatsMap[
+                response.recipientEmail || response.recipient].areSubQuestionChosenWeightsAllNull[i] = false;
+        }
       }
     }
 
     for (const recipient of Object.keys(this.perRecipientStatsMap)) {
       const perRecipientStats: PerRecipientStats = this.perRecipientStatsMap[recipient];
 
       // Answers sum = number of answers in each column
-      perRecipientStats.answersSum = this.calculateAnswersSum(perRecipientStats.answers);
+      perRecipientStats.answersSum = this.sumValidValuesByColumn(perRecipientStats.answers);
       perRecipientStats.percentages = this.calculatePercentages(perRecipientStats.answers);
       perRecipientStats.percentagesAverage = this.calculatePercentagesAverage(perRecipientStats.answersSum);
+      perRecipientStats.subQuestionTotalChosenWeight =
+          perRecipientStats.subQuestionTotalChosenWeight.map((val: number, i: number) =>
+              (perRecipientStats.areSubQuestionChosenWeightsAllNull[i] ? NO_VALUE : val));
       perRecipientStats.subQuestionWeightAverage =
           this.calculateSubQuestionWeightAverage(perRecipientStats.answers);
       perRecipientStats.weightsAverage = this.calculateWeightsAverage(this.weights);
-      // Overall weighted sum = sum of total chosen weight for all sub questions
-      perRecipientStats.overallWeightedSum =
-        +(perRecipientStats.subQuestionTotalChosenWeight.reduce((a, b) => a + b)).toFixed(2);
-      // Overall weighted average = overall weighted sum / total number of responses
-      perRecipientStats.overallWeightAverage = +(perRecipientStats.overallWeightedSum
-          / this.calculateNumResponses(perRecipientStats.answersSum)).toFixed(2);
+      perRecipientStats.overallWeightedSum = this.calculateOverallWeightedSum(
+          perRecipientStats.areSubQuestionChosenWeightsAllNull, perRecipientStats.subQuestionTotalChosenWeight);
+      // Overall weighted average = overall weighted sum / total number of responses with non-null weights
+      perRecipientStats.overallWeightAverage = perRecipientStats.overallWeightedSum === NO_VALUE
+          ? NO_VALUE
+          : +(perRecipientStats.overallWeightedSum
+              / this.calculateNumResponses(this.countResponsesByRowWithValidWeight(perRecipientStats.answers)))
+              .toFixed(2);
     }
   }
 
+  // Number of responses for each sub question with non-null weights
+  private countResponsesByRowWithValidWeight(answers: number[][]): number[] {
+    const sums: number[] = [];
+    for (let r: number = 0; r < answers.length; r += 1) {
+        let sum: number = 0;
+        for (let c: number = 0; c < answers[0].length; c += 1) {
+            if (this.weights[r][c] === null) {
+                continue;
+            }
+            sum += answers[r][c];
+        }
+        sums[r] = sum;
+    }
+    return sums;
+  }
+
   private calculateSubQuestionWeightAverage(answers: number[][]): number[] {
-    const sums: number[] = answers.map((weightedAnswers: number[]) =>
-        weightedAnswers.reduce((a: number, b: number) => a + b, 0));
+    const sums: number[] = this.countResponsesByRowWithValidWeight(answers);
 
     return answers.map((subQuestionAnswer: number[], subQuestionIdx: number): number => {
-      const weightAverage: number = sums[subQuestionIdx] === 0 ? 0
-          : subQuestionAnswer.reduce((prevValue: number, currValue: number, currentIndex: number): number =>
-              prevValue + currValue * this.weights[subQuestionIdx][currentIndex], 0) / sums[subQuestionIdx];
+      if (sums[subQuestionIdx] === 0) {
+        return NO_VALUE;
+      }
+      const weightAverage: number =
+          subQuestionAnswer.reduce((prevValue: number, currValue: number, currentIndex: number): number =>
+              (this.weights[subQuestionIdx][currentIndex] === null
+                  ? prevValue
+                  : prevValue + currValue * this.weights[subQuestionIdx][currentIndex]), 0) / sums[subQuestionIdx];
       return +weightAverage.toFixed(2);
     });
   }
@@ -176,27 +207,40 @@ export class RubricQuestionStatisticsCalculation
     return percentages;
   }
 
-  // Calculate sum of answers for each column
-  private calculateAnswersSum(answers: number[][]): number[] {
+  // Calculate sum of non-null values for each column
+  private sumValidValuesByColumn(matrix: number[][]): number[] {
     const sums: number[] = [];
-    for (let i: number = 0; i < answers[0].length; i += 1) {
+    for (let c: number = 0; c < matrix[0].length; c += 1) {
       let sum: number = 0;
-      for (let j: number = 0; j < answers.length; j += 1) {
-        sum += answers[j][i];
+      for (let r: number = 0; r < matrix.length; r += 1) {
+        sum += matrix[r][c] === null ? 0 : matrix[r][c];
       }
-      sums[i] = sum;
+      sums[c] = sum;
     }
     return sums;
   }
 
-  // Calculate weight average for each column
+  // Count number of non-null values for each column
+  private countValidValuesByColumn(matrix: number[][]): number[] {
+    const counts: number[] = [];
+    for (let c: number = 0; c < matrix[0].length; c += 1) {
+      let count: number = 0;
+      for (let r: number = 0; r < matrix.length; r += 1) {
+        count += matrix[r][c] === null ? 0 : 1;
+      }
+      counts[c] = count;
+    }
+    return counts;
+  }
+
+  // Calculate non-null weight average for each column
   private calculateWeightsAverage(weights: number[][]): number[] {
-    // Calculate sum of weights for each column
-    const sums: number[] = this.calculateAnswersSum(weights);
+    const sums: number[] = this.sumValidValuesByColumn(weights);
+    const counts: number[] = this.countValidValuesByColumn(weights);
     const averages: number[] = [];
-    // Divide each weight sum by number of weights
+    // Divide each weight sum by number of non-null weights
     for (let i: number = 0; i < sums.length; i += 1) {
-      averages[i] = +(sums[i] / weights.length).toFixed(2);
+      averages[i] = counts[i] ? +(sums[i] / counts[i]).toFixed(2) : NO_VALUE;
     }
     return averages;
   }
@@ -217,4 +261,16 @@ export class RubricQuestionStatisticsCalculation
   private calculateNumResponses(answersSum: number[]): number {
     return answersSum.reduce((a, b) => a + b);
   }
+
+  // Overall weighted sum is sum of total chosen non-null weight for all sub questions
+  private calculateOverallWeightedSum(areChosenWeightsAllNull: boolean[], totalChosenWeights: number[]): number {
+    if (areChosenWeightsAllNull.every(Boolean)) {
+        return NO_VALUE;
+    }
+    let sum: number = 0;
+    for (const totalChosenWeight of totalChosenWeights) {
+        sum += totalChosenWeight === NO_VALUE ? 0 : totalChosenWeight;
+    }
+    return +(sum).toFixed(2);
+  }
 }
diff --git a/...app/components/question-types/question-statistics/rubric-question-statistics.component.ts b/...app/components/question-types/question-statistics/rubric-question-statistics.component.ts
@@ -1,6 +1,7 @@
 import { Component, OnChanges } from '@angular/core';
 import { StringHelper } from '../../../../services/string-helper';
 import { DEFAULT_RUBRIC_QUESTION_DETAILS } from '../../../../types/default-question-structs';
+import { NO_VALUE } from '../../../../types/feedback-response-details';
 import { SortBy } from '../../../../types/sort-properties';
 import { ColumnData, SortableTableCellData } from '../../sortable-table/sortable-table.component';
 import {
@@ -54,21 +55,29 @@ export class RubricQuestionStatisticsComponent extends RubricQuestionStatisticsC
             return {
               value: `${this.percentagesExcludeSelf[questionIndex][choiceIndex]}%`
                   + ` (${this.answersExcludeSelf[questionIndex][choiceIndex]})`
-                  + `${this.isWeightStatsVisible ? ` [${this.weights[questionIndex][choiceIndex]}]` : ''}`,
+                  + `${this.isWeightStatsVisible
+                      ? ` [${this.getDisplayWeight(this.weights[questionIndex][choiceIndex])}]`
+                      : ''}`,
             };
           }
           return {
             value: `${this.percentages[questionIndex][choiceIndex]}%`
                 + ` (${this.answers[questionIndex][choiceIndex]})`
-                + `${this.isWeightStatsVisible ? ` [${this.weights[questionIndex][choiceIndex]}]` : ''}`,
+                + `${this.isWeightStatsVisible
+                    ? ` [${this.getDisplayWeight(this.weights[questionIndex][choiceIndex])}]`
+                    : ''}`,
           };
         }),
       ];
       if (this.isWeightStatsVisible) {
         if (this.excludeSelf) {
-          currRow.push({ value: this.subQuestionWeightAverageExcludeSelf[questionIndex] });
+          currRow.push({
+            value: this.getDisplayWeight(this.subQuestionWeightAverageExcludeSelf[questionIndex]),
+          });
         } else {
-          currRow.push({ value: this.subQuestionWeightAverage[questionIndex] });
+          currRow.push({
+            value: this.getDisplayWeight(this.subQuestionWeightAverage[questionIndex]),
+          });
         }
       }
 
@@ -103,11 +112,11 @@ export class RubricQuestionStatisticsComponent extends RubricQuestionStatisticsC
             return {
               value: `${perRecipientStats.percentages[questionIndex][choiceIndex]}%`
                   + ` (${perRecipientStats.answers[questionIndex][choiceIndex]})`
-                  + ` [${this.weights[questionIndex][choiceIndex]}]`,
+                  + ` [${this.getDisplayWeight(this.weights[questionIndex][choiceIndex])}]`,
             };
           }),
-          { value: perRecipientStats.subQuestionTotalChosenWeight[questionIndex] },
-          { value: perRecipientStats.subQuestionWeightAverage[questionIndex] },
+          { value: this.getDisplayWeight(perRecipientStats.subQuestionTotalChosenWeight[questionIndex]) },
+          { value: this.getDisplayWeight(perRecipientStats.subQuestionWeightAverage[questionIndex]) },
         ]);
       });
     });
@@ -124,6 +133,9 @@ export class RubricQuestionStatisticsComponent extends RubricQuestionStatisticsC
 
     this.perRecipientOverallRowsData = [];
     Object.values(this.perRecipientStatsMap).forEach((perRecipientStats: PerRecipientStats) => {
+      const perCriterionAverage: string =
+          perRecipientStats.subQuestionWeightAverage.map((val: number) =>
+          this.getDisplayWeight(val)).toString();
       this.perRecipientOverallRowsData.push([
         { value: perRecipientStats.recipientTeam },
         { value: perRecipientStats.recipientName },
@@ -132,13 +144,17 @@ export class RubricQuestionStatisticsComponent extends RubricQuestionStatisticsC
           return {
             value: `${perRecipientStats.percentagesAverage[choiceIndex]}%`
                 + ` (${perRecipientStats.answersSum[choiceIndex]})`
-                + ` [${perRecipientStats.weightsAverage[choiceIndex]}]`,
+                + ` [${this.getDisplayWeight(perRecipientStats.weightsAverage[choiceIndex])}]`,
           };
         }),
-        { value: perRecipientStats.overallWeightedSum },
-        { value: perRecipientStats.overallWeightAverage },
-        { value: perRecipientStats.subQuestionWeightAverage.toString() },
+        { value: this.getDisplayWeight(perRecipientStats.overallWeightedSum) },
+        { value: this.getDisplayWeight(perRecipientStats.overallWeightAverage) },
+        { value: perCriterionAverage },
       ]);
     });
   }
+
+  private getDisplayWeight(weight: number): any {
+    return weight === null || weight === NO_VALUE ? '-' : weight;
+  }
 }
diff --git a/.../app/components/question-types/question-statistics/test-data/rubricQuestionResponses.json b/.../app/components/question-types/question-statistics/test-data/rubricQuestionResponses.json
@@ -70,6 +70,7 @@
       "recipientEmail": "alice@gmail.com",
       "recipientName": "Alice",
       "recipientTeam": "Team 1",
+      "areSubQuestionChosenWeightsAllNull": [false, false, false],
       "subQuestionTotalChosenWeight": [1, 1, 0.8],
       "subQuestionWeightAverage": [0.5, 0.5, 0.4],
       "weightsAverage": [0.23, 0.77],
@@ -88,6 +89,7 @@
       "recipientEmail": "bob@gmail.com",
       "recipientName": "Bob",
       "recipientTeam": "Team 2",
+      "areSubQuestionChosenWeightsAllNull": [false, false, false],
       "subQuestionTotalChosenWeight": [0.4, 1, 0.8],
       "subQuestionWeightAverage": [0.2, 0.5, 0.4],
       "weightsAverage": [0.23, 0.77],