From 61c7a6c03bac91dbe183437e00937c5414bf197a Mon Sep 17 00:00:00 2001 From: Thomas Sparks <69657545+thsparks@users.noreply.github.com> Date: Fri, 12 Apr 2024 12:15:07 -0700 Subject: [PATCH] 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. --- teachertool/src/services/loggingService.ts | 5 +++-- teachertool/src/services/makecodeEditorService.ts | 4 ++-- teachertool/src/transforms/getRubricFromFileAsync.ts | 2 +- .../src/transforms/loadProjectMetadataAsync.ts | 2 +- teachertool/src/transforms/runEvaluateAsync.ts | 11 ++++++++--- teachertool/src/transforms/setActiveTab.ts | 1 - .../src/transforms/tryLoadLastActiveRubricAsync.ts | 2 +- 7 files changed, 16 insertions(+), 11 deletions(-) diff --git a/teachertool/src/services/loggingService.ts b/teachertool/src/services/loggingService.ts index c52fa2307c6a..4a969408b661 100644 --- 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 index a1b5a66d8326..73ff6ff083eb 100644 --- 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 index a9c94e278378..e516d520d23d 100644 --- 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 index 847b81bcd1c2..400effff4cc7 100644 --- 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 index 6bc99d05226a..602428044db8 100644 --- 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 index 87e34e9ff53a..71df53b84c6c 100644 --- 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 index 3bf907878113..8fe4f661af68 100644 --- 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);