From e6aea4e7068ba828a40fb07e608557ba7ea3f0c3 Mon Sep 17 00:00:00 2001
From: Jack Greenlee <greenljk@mail.uc.edu>
Date: Mon, 19 Aug 2024 12:45:10 -0400
Subject: [PATCH] fix agg metrics, don't use "for .. in .." on JS arrays

Agg metrics were broken because we iterated over an array using "for .. in ..", which has unexpected consequences.

"for .. in .." iterates over all the keys in an object. Arrays are a type of object, where the elements of the array are keyed by index.
But they can have other properties too (like their methods) so when you use "for .. in .." you get everything.

In general arrays should be iterated with "for .. of .." or ".forEach", but in this specific case I noticed we are basically just copying an array with some values substituted, so I opted for ".map".

Note: There is repeated code across these two files that should obviously be refactored later, but for now I just want to get this bug fixed.
---
 www/js/metrics/CarbonFootprintCard.tsx | 13 ++++++-------
 www/js/metrics/CarbonTextCard.tsx      | 13 ++++++-------
 2 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/www/js/metrics/CarbonFootprintCard.tsx b/www/js/metrics/CarbonFootprintCard.tsx
index 9624e10df..c40254256 100644
--- a/www/js/metrics/CarbonFootprintCard.tsx
+++ b/www/js/metrics/CarbonFootprintCard.tsx
@@ -131,15 +131,14 @@ const CarbonFootprintCard = ({ userMetrics, aggMetrics }: Props) => {
 
       // Issue 422:
       // https://github.com/e-mission/e-mission-docs/issues/422
-      let aggCarbonData: MetricsSummary[] = [];
-      for (let i in aggThisWeekSummary) {
-        aggCarbonData.push(aggThisWeekSummary[i]);
-        if (isNaN(aggCarbonData[i].values)) {
+      let aggCarbonData: MetricsSummary[] = aggThisWeekSummary.map((summaryEntry) => {
+        if (isNaN(summaryEntry.values)) {
           logWarn(`WARNING in calculating groupCarbonRecords: value is NaN for mode 
-            ${aggCarbonData[i].key}, changing to 0`);
-          aggCarbonData[i].values = 0;
+            ${summaryEntry.key}, changing to 0`);
+          summaryEntry.values = 0;
         }
-      }
+        return summaryEntry;
+      });
 
       let groupRecords: { label: string; x: number | string; y: number | string }[] = [];
 
diff --git a/www/js/metrics/CarbonTextCard.tsx b/www/js/metrics/CarbonTextCard.tsx
index ca9f50fdc..225942af1 100644
--- a/www/js/metrics/CarbonTextCard.tsx
+++ b/www/js/metrics/CarbonTextCard.tsx
@@ -107,15 +107,14 @@ const CarbonTextCard = ({ userMetrics, aggMetrics }: Props) => {
 
       // Issue 422:
       // https://github.com/e-mission/e-mission-docs/issues/422
-      let aggCarbonData: MetricsSummary[] = [];
-      for (let i in aggThisWeekSummary) {
-        aggCarbonData.push(aggThisWeekSummary[i]);
-        if (isNaN(aggCarbonData[i].values)) {
+      let aggCarbonData: MetricsSummary[] = aggThisWeekSummary.map((summaryEntry) => {
+        if (isNaN(summaryEntry.values)) {
           logWarn(`WARNING in calculating groupCarbonRecords: value is NaN for mode 
-            ${aggCarbonData[i].key}, changing to 0`);
-          aggCarbonData[i].values = 0;
+            ${summaryEntry.key}, changing to 0`);
+          summaryEntry.values = 0;
         }
-      }
+        return summaryEntry;
+      });
 
       let groupText: { label: string; value: string }[] = [];