Teacher Tool: Fix Parameters on Criteria Instances that Reference the…

… Same Validator Plans (#9967) While simplifying the catalog, I noticed parameters were overwriting each other when we had multiple criteria instances that referenced the same validator plan. This was happening because all of them were pointing to the same plan object. To fix it, we can make a deep copy of the validator plan before we modify it. I also updated the logDebug function to accept a data parameter to help keep our logs a little tidier. But the main change is in runEvaluateAsync.ts.
microsoft · Apr 12, 2024 · 61c7a6c · 61c7a6c
1 parent ab7902d
commit 61c7a6c
Show file tree

Hide file tree

Showing 7 changed files with 16 additions and 11 deletions.
diff --git a/teachertool/src/services/loggingService.ts b/teachertool/src/services/loggingService.ts
@@ -32,8 +32,9 @@ export const logInfo = (message: any) => {
     console.log(timestamp(), message);
 };
 
-export const logDebug = (message: any) => {
+export const logDebug = (message: any, data?: any) => {
     if (pxt.BrowserUtils.isLocalHost() || pxt.options.debug) {
-        console.log(timestamp(), message);
+        console.log(timestamp(), message, data);
     }
 };
+
diff --git a/teachertool/src/services/makecodeEditorService.ts b/teachertool/src/services/makecodeEditorService.ts
@@ -21,10 +21,10 @@ export function setEditorRef(ref: HTMLIFrameElement | undefined) {
         driver = new EditorDriver(ref);
 
         driver.addEventListener("message", ev => {
-            logDebug(`Message received from iframe: ${JSON.stringify(ev)}`);
+            logDebug(`Message received from iframe. ID: ${ev?.id}`, ev);
         });
         driver.addEventListener("sent", ev => {
-            logDebug(`Sent message to iframe: ${JSON.stringify(ev)}`);
+            logDebug(`Sent message to iframe. ID: ${ev?.id}`, ev);
         });
         driver.addEventListener("editorcontentloaded", ev => {
             AutorunService.poke();

diff --git a/teachertool/src/transforms/getRubricFromFileAsync.ts b/teachertool/src/transforms/getRubricFromFileAsync.ts
@@ -7,7 +7,7 @@ export async function getRubricFromFileAsync(file: File, allowPartial: boolean):
     let rubric = await loadRubricFromFileAsync(file);
 
     if (rubric) {
-        logDebug(`Loading rubric '${rubric.name}' from file...`);
+        logDebug("Loading rubric from file...", {file, rubric});
 
         const rubricVerificationResult = verifyRubricIntegrity(rubric);
 

diff --git a/teachertool/src/transforms/loadProjectMetadataAsync.ts b/teachertool/src/transforms/loadProjectMetadataAsync.ts
@@ -34,5 +34,5 @@ export async function loadProjectMetadataAsync(inputText: string, shareLink: str
     };
     dispatch(Actions.setProjectMetadata(projectData));
     initNewProjectResults();
-    logDebug(`Loaded project metadata: ${JSON.stringify(projMeta)}`);
+    logDebug("Loaded project metadata", projMeta);
 }
diff --git a/teachertool/src/transforms/runEvaluateAsync.ts b/teachertool/src/transforms/runEvaluateAsync.ts
@@ -1,4 +1,4 @@
-import { logError } from "../services/loggingService";
+import { logDebug, logError } from "../services/loggingService";
 import { runValidatorPlanAsync } from "../services/makecodeEditorService";
 import { stateAndDispatch } from "../state";
 import * as Actions from "../state/actions";
@@ -28,14 +28,17 @@ function generateValidatorPlan(
         return undefined;
     }
 
-    const plan = teacherTool.validatorPlans?.find(plan => plan.name === catalogCriteria.use);
-    if (!plan) {
+    const planTemplate = teacherTool.validatorPlans?.find(plan => plan.name === catalogCriteria.use);
+    if (!planTemplate) {
         logError(ErrorCode.evalMissingPlan, "Attempting to evaluate criteria with unrecognized plan", {
             plan: catalogCriteria.use,
         });
         return undefined;
     }
 
+    // Create a copy we can modify without affecting other uses of this template.
+    const plan = pxt.Util.deepCopy(planTemplate);
+
     // Fill in parameters.
     for (const param of criteriaInstance.params ?? []) {
         const catalogParam = catalogCriteria.params?.find(p => p.name === param.name);
@@ -109,6 +112,8 @@ export async function runEvaluateAsync(fromUserInteraction: boolean) {
 
                 const plan = generateValidatorPlan(criteriaInstance, fromUserInteraction);
 
+                logDebug(`${criteriaInstance.instanceId}: Generated Plan`, plan)
+
                 if (!plan) {
                     dispatch(Actions.clearEvalResult(criteriaInstance.instanceId));
                     return resolve(false);

diff --git a/teachertool/src/transforms/setActiveTab.ts b/teachertool/src/transforms/setActiveTab.ts
@@ -1,4 +1,3 @@
-import { logDebug } from "../services/loggingService";
 import { stateAndDispatch } from "../state";
 import * as Actions from "../state/actions";
 import { TabName } from "../types";

diff --git a/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts b/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts
@@ -9,7 +9,7 @@ export async function tryLoadLastActiveRubricAsync() {
     const lastActiveRubric = await getLastActiveRubricAsync();
 
     if (lastActiveRubric) {
-        logDebug(`Loading last active rubric '${lastActiveRubric.name}'...`);
+        logDebug("Loading last active rubric...", lastActiveRubric);
 
         const rubricVerificationResult = verifyRubricIntegrity(lastActiveRubric);