Fix bug with calculating quality evaluation

src/main/java/no/ndla/taxonomy/domain/GradeAverage.java

@@ -11,23 +11,23 @@
 import java.util.Optional;
 
 public class GradeAverage {
-    public GradeAverage(double averageValue, int count) {
-        this.averageValue = averageValue;
+    public GradeAverage(int averageSum, int count) {
+        this.averageSum = averageSum;
         this.count = count;
     }
 
-    double averageValue;
+    int averageSum;
     int count;
 
     public static GradeAverage fromGrades(Collection<Optional<Grade>> grades) {
         var existing = grades.stream().flatMap(Optional::stream).toList();
         var count = existing.size();
-        var avg = existing.stream().mapToInt(Grade::toInt).average().orElse(0.0);
-        return new GradeAverage(avg, count);
+        var sum = existing.stream().mapToInt(Grade::toInt).sum();
+        return new GradeAverage(sum, count);
     }
 
-    public double getAverageValue() {
-        return averageValue;
+    public int getAverageSum() {
+        return averageSum;
     }
 
     public int getCount() {
@@ -44,11 +44,15 @@ public boolean equals(Object obj) {
         }
 
         GradeAverage that = (GradeAverage) obj;
-        return Double.compare(that.averageValue, averageValue) == 0 && count == that.count;
+        return averageSum == that.averageSum && count == that.count;
     }
 
     @Override
     public String toString() {
-        return "GradeAverage{averageValue=" + averageValue + ", count=" + count + '}';
+        return "GradeAverage{averageSum=" + averageSum + ", count=" + count + '}';
+    }
+
+    public double getAverageValue() {
+        return (double) averageSum / count;
     }
 }

src/main/java/no/ndla/taxonomy/domain/Node.java

@@ -83,8 +83,8 @@ public class Node extends DomainObject implements EntityWithMetadata {
     @Column(name = "quality_evaluation_comment")
     private String qualityEvaluationComment;
 
-    @Column(name = "child_quality_evaluation_average")
-    private Double childQualityEvaluationAverage;
+    @Column(name = "child_quality_evaluation_sum")
+    private int childQualityEvaluationSum;
 
     @Column(name = "child_quality_evaluation_count")
     private int childQualityEvaluationCount;
@@ -147,25 +147,33 @@ public Optional<String> getQualityEvaluationNote() {
     }
 
     public Optional<GradeAverage> getChildQualityEvaluationAverage() {
-        return Optional.ofNullable(this.childQualityEvaluationAverage)
-                .map(avg -> new GradeAverage(avg, this.childQualityEvaluationCount));
+        if (this.childQualityEvaluationSum == 0) {
+            return Optional.empty();
+        }
+        var gradeAverage = new GradeAverage(this.childQualityEvaluationSum, this.childQualityEvaluationCount);
+        return Optional.of(gradeAverage);
     }
 
     public void addGradeAverageTreeToAverageCalculation(GradeAverage newGradeAverage) {
         var childAvg = getChildQualityEvaluationAverage();
         if (childAvg.isEmpty()) {
-            this.childQualityEvaluationAverage = newGradeAverage.getAverageValue();
+            this.childQualityEvaluationSum = newGradeAverage.getAverageSum();
             this.childQualityEvaluationCount = newGradeAverage.getCount();
             return;
         }
 
-        var oldSum = childAvg.get().getAverageValue() * childAvg.get().getCount();
-        var sumToAdd = newGradeAverage.getAverageValue() * newGradeAverage.getCount();
+        var oldSum = childAvg.get().getAverageSum();
+        var sumToAdd = newGradeAverage.getAverageSum();
         var newSum = oldSum + sumToAdd;
         var newCount = childAvg.get().getCount() + newGradeAverage.getCount();
 
-        this.childQualityEvaluationAverage = newSum / newCount;
-        this.childQualityEvaluationCount = newCount;
+        if (newCount > 0) {
+            this.childQualityEvaluationSum = newSum;
+            this.childQualityEvaluationCount = newCount;
+        } else {
+            this.childQualityEvaluationSum = 0;
+            this.childQualityEvaluationCount = 0;
+        }
     }
 
     public void removeGradeAverageTreeFromAverageCalculation(GradeAverage previousGradeAverage) {
@@ -178,17 +186,17 @@ public void removeGradeAverageTreeFromAverageCalculation(GradeAverage previousGr
             return;
         }
 
-        var totalSum = childAvg.get().getAverageValue() * childAvg.get().getCount();
-        var sumToRemove = previousGradeAverage.getAverageValue() * previousGradeAverage.getCount();
+        var totalSum = childAvg.get().getAverageSum();
+        var sumToRemove = previousGradeAverage.getAverageSum();
 
         var newSum = totalSum - sumToRemove;
         var newCount = childAvg.get().getCount() - previousGradeAverage.getCount();
 
         if (newCount == 0) {
-            this.childQualityEvaluationAverage = null;
+            this.childQualityEvaluationSum = 0;
             this.childQualityEvaluationCount = 0;
         } else {
-            this.childQualityEvaluationAverage = newSum / newCount;
+            this.childQualityEvaluationSum = newSum;
             this.childQualityEvaluationCount = newCount;
         }
     }
@@ -197,42 +205,30 @@ public void updateChildQualityEvaluationAverage(Optional<Grade> previousGrade, O
         var childAvg = getChildQualityEvaluationAverage();
         if (childAvg.isEmpty()) {
             newGrade.ifPresent(ng -> {
-                this.childQualityEvaluationAverage = (double) ng.toInt();
+                this.childQualityEvaluationSum = ng.toInt();
                 this.childQualityEvaluationCount = 1;
             });
             return;
         }
 
         var avg = childAvg.get();
-
-        if (Double.isNaN(avg.averageValue)) {
-            logger.warn(
-                    "Child quality evaluation average of node '{}' is NaN. Recalculating entire tree.",
-                    this.getPublicId());
-            updateEntireAverageTree();
-            return;
-        }
-
         if (previousGrade.isEmpty() && newGrade.isEmpty()) return;
         else if (previousGrade.isEmpty()) { // New grade is present
             var newCount = avg.getCount() + 1;
-            var newSum = ((avg.averageValue * avg.getCount()) + newGrade.get().toInt());
-            var newAverage = newSum / newCount;
+            var newSum = avg.averageSum + newGrade.get().toInt();
             this.childQualityEvaluationCount = newCount;
-            this.childQualityEvaluationAverage = newAverage;
+            this.childQualityEvaluationSum = newSum;
         } else if (newGrade.isEmpty()) { // Previous grade is present
             var newCount = avg.getCount() - 1;
-            var oldSum = (avg.averageValue * avg.getCount());
+            var oldSum = avg.getAverageSum();
             var newSum = oldSum - previousGrade.get().toInt();
-            var newAverage = newCount > 0 ? newSum / newCount : null;
             this.childQualityEvaluationCount = newCount;
-            this.childQualityEvaluationAverage = newAverage;
+            this.childQualityEvaluationSum = newSum;
         } else { // Both grades are present
-            var oldSum = avg.averageValue * avg.getCount();
+            var oldSum = avg.getAverageSum();
             var newSum = oldSum - previousGrade.get().toInt() + newGrade.get().toInt();
-            var newAverage = newSum / avg.getCount();
             this.childQualityEvaluationCount = avg.getCount();
-            this.childQualityEvaluationAverage = newAverage;
+            this.childQualityEvaluationSum = newSum;
         }
     }
 
@@ -246,10 +242,10 @@ public void updateEntireAverageTree() {
                 gradeAverage);
 
         if (gradeAverage.count == 0) {
-            this.childQualityEvaluationAverage = null;
+            this.childQualityEvaluationSum = 0;
             this.childQualityEvaluationCount = 0;
         } else if (gradeAverage.count > 0) {
-            this.childQualityEvaluationAverage = gradeAverage.averageValue;
+            this.childQualityEvaluationSum = gradeAverage.getAverageSum();
             this.childQualityEvaluationCount = gradeAverage.count;
         }
     }

src/main/java/no/ndla/taxonomy/domain/UpdateOrDelete.java

@@ -28,6 +28,10 @@ public static <T> UpdateOrDelete<T> Default() {
         return new UpdateOrDelete<>();
     }
 
+    public static <T> UpdateOrDelete<T> Update(T value) {
+        return new UpdateOrDelete<>(value, false);
+    }
+
     public boolean isDelete() {
         return delete;
     }

src/main/java/no/ndla/taxonomy/repositories/NodeRepository.java

@@ -47,11 +47,11 @@ public interface NodeRepository extends TaxonomyRepository<Node> {
             """)
     Stream<Node> findNodesWithQualityEvaluation();
 
-    @Modifying
+    @Modifying(flushAutomatically = true, clearAutomatically = true)
     @Query(
             """
             UPDATE Node n
-            SET n.childQualityEvaluationAverage = NULL,
+            SET n.childQualityEvaluationSum = 0,
                 n.childQualityEvaluationCount = 0
             """)
     void wipeQualityEvaluationAverages();

src/main/java/no/ndla/taxonomy/rest/v1/CrudController.java

@@ -76,7 +76,7 @@ protected void deleteEntity(@PathVariable("id") URI id) {
 
         if (parents.isPresent()) {
             var p = parents.get();
-            qualityEvaluationService.updateQualityEvaluationOf(p, oldGrade, Optional.empty());
+            qualityEvaluationService.updateQualityEvaluationOfRecursive(p, oldGrade, Optional.empty());
         }
     }
 

src/main/java/no/ndla/taxonomy/service/QualityEvaluationService.java

@@ -96,17 +96,11 @@ protected void updateQualityEvaluationOfParents(Node node, Optional<Grade> oldGr
             return;
         }
 
-        updateQualityEvaluationOf(node.getParentNodes(), oldGrade, newGrade);
+        updateQualityEvaluationOfRecursive(node.getParentNodes(), oldGrade, newGrade);
     }
 
     @Transactional
-    public void updateQualityEvaluationOf(
-            Collection<Node> parents, Optional<Grade> oldGrade, Optional<Grade> newGrade) {
-        updateQualityEvaluationOfRecursive(parents, oldGrade, newGrade);
-    }
-
-    @Transactional
-    protected void updateQualityEvaluationOfRecursive(
+    public void updateQualityEvaluationOfRecursive(
             Collection<Node> parents, Optional<Grade> oldGrade, Optional<Grade> newGrade) {
         var updatedParents = parents.stream()
                 .peek(p -> {

src/main/java/no/ndla/taxonomy/service/dtos/GradeAverageDTO.java

@@ -29,7 +29,8 @@ public static double roundToSingleDecimal(double value) {
 
     public static Optional<GradeAverageDTO> fromNode(Node node) {
         return node.getChildQualityEvaluationAverage().map(ga -> {
-            var roundedAvg = roundToSingleDecimal(ga.getAverageValue());
+            var avg = (double) ga.getAverageSum() / ga.getCount();
+            var roundedAvg = roundToSingleDecimal(avg);
             var count1 = ga.getCount();
             return new GradeAverageDTO(roundedAvg, count1);
         });

src/main/resources/db-master-changelog.xml

@@ -1602,4 +1602,11 @@
         </sql>
     </changeSet>
 
+    <changeSet id="20250120 Quality evaluation sum instead of calculated" author="Jonas Natten">
+        <dropColumn tableName="node" columnName="child_quality_evaluation_average"/>
+        <addColumn tableName="node">
+            <column name="child_quality_evaluation_sum" type="int" defaultValueNumeric="0" />
+        </addColumn>
+    </changeSet>
+
 </databaseChangeLog>