From 1e22ec7a07a5979d06b050bff407741a6e987c08 Mon Sep 17 00:00:00 2001 From: Thomas Sparks <69657545+thsparks@users.noreply.github.com> Date: Fri, 20 Sep 2024 15:16:49 -0700 Subject: [PATCH] Teacher Tool: More Telemetry (#10196) This change adds a few telemetry events and some code changes to support them: 1. Changing the eval result (including a field indicating whether or not it's manual and whether the previous result was manual, which necessitated adding resultIsManual to the result type. I think this could be helpful in the future too, if we decide not to overwrite manual results when doing bulk evaluate) 2. Changing eval notes (debounced) 3. Run single eval (including hash of the checklist which should help with understanding evals / checklist and usage of pre-made checklists) 4. Run bulk eval 5. Importing a checklist (whether there's an invalid file, a successful import, or closed without doing anything) 6. Loading in new projects 7. Block picker opened & block selected 8. Adjusted logging of opening pre-built checklists so we can include checklist hash (and error reporting if a pre-built checklist is invalid) There's also a one-line bug fix so we don't show the "Replace existing checklist" warning when the user clicks new checklist for the first time (only consider having an "existing checklist" if there is criteria, regardless of name). --- teachertool/src/App.tsx | 2 ++ .../src/components/BlockPickerModal.tsx | 3 +- teachertool/src/components/CatalogOverlay.tsx | 10 ++---- .../components/CriteriaEvalResultDropdown.tsx | 2 +- .../components/CriteriaInstanceDisplay.tsx | 3 +- .../src/components/CriteriaResultEntry.tsx | 15 ++++++-- .../src/components/EvalResultDisplay.tsx | 12 +++++-- teachertool/src/components/HomeScreen.tsx | 3 +- .../src/components/ImportChecklistModal.tsx | 15 +++++--- teachertool/src/components/ShareLinkInput.tsx | 12 ++++++- teachertool/src/constants.ts | 18 ++++++++-- .../src/services/makecodeEditorService.ts | 11 ++++++ teachertool/src/state/helpers.ts | 2 +- .../src/transforms/loadChecklistAsync.ts | 17 ++++++---- teachertool/src/transforms/mergeEvalResult.ts | 8 ++++- .../src/transforms/runSingleEvaluateAsync.ts | 6 ++-- teachertool/src/transforms/setEvalResult.ts | 34 ++++++++++++++++++- .../src/transforms/setEvalResultOutcome.ts | 5 +-- .../src/transforms/setParameterValue.ts | 2 +- teachertool/src/transforms/setRunOnLoad.ts | 1 - teachertool/src/types/criteria.ts | 1 + teachertool/src/types/criteriaParameters.ts | 23 ++++++++----- teachertool/src/types/errorCode.ts | 1 + teachertool/src/utils/index.ts | 11 ++++++ 24 files changed, 167 insertions(+), 50 deletions(-) diff --git a/teachertool/src/App.tsx b/teachertool/src/App.tsx index 8698d348668b..15f7df9cbe62 100644 --- a/teachertool/src/App.tsx +++ b/teachertool/src/App.tsx @@ -19,6 +19,7 @@ import { SignedOutPanel } from "./components/SignedOutPanel"; import * as authClient from "./services/authClient"; import { ErrorCode } from "./types/errorCode"; import { loadProjectMetadataAsync } from "./transforms/loadProjectMetadataAsync"; +import { Ticks } from "./constants"; export const App = () => { const { state, dispatch } = useContext(AppStateContext); @@ -47,6 +48,7 @@ export const App = () => { const decoded = decodeURIComponent(projectParam); const shareId = pxt.Cloud.parseScriptId(decoded); if (!!shareId) { + pxt.tickEvent(Ticks.LoadProjectFromUrl); await loadProjectMetadataAsync(decoded, shareId); } } diff --git a/teachertool/src/components/BlockPickerModal.tsx b/teachertool/src/components/BlockPickerModal.tsx index 8d0c1c811abe..1775e2d780ed 100644 --- a/teachertool/src/components/BlockPickerModal.tsx +++ b/teachertool/src/components/BlockPickerModal.tsx @@ -11,7 +11,7 @@ import { getReadableBlockString } from "../utils"; import { setParameterValue } from "../transforms/setParameterValue"; import { ErrorCode } from "../types/errorCode"; import { logError } from "../services/loggingService"; -import { Strings } from "../constants"; +import { Strings, Ticks } from "../constants"; import { BlockPickerOptions } from "../types/modalOptions"; import css from "./styling/BlockPickerModal.module.scss"; @@ -51,6 +51,7 @@ const BlockPickerCategory: React.FC = ({ category, onB const [expanded, setExpanded] = useState(false); function blockSelected(block: pxt.editor.ToolboxBlockDefinition) { + pxt.tickEvent(Ticks.BlockPickerBlockSelected, { blockId: block.blockId ?? "" }); onBlockSelected?.(block); } diff --git a/teachertool/src/components/CatalogOverlay.tsx b/teachertool/src/components/CatalogOverlay.tsx index ecd99ea0ff92..f86fc80d5ff1 100644 --- a/teachertool/src/components/CatalogOverlay.tsx +++ b/teachertool/src/components/CatalogOverlay.tsx @@ -44,10 +44,7 @@ const CatalogItemLabel: React.FC = ({ catalogCriteria, is
{isMaxed ? ( - + ) : ( <> = ({ catalogCriteria, is title={lf("Added!")} /> diff --git a/teachertool/src/components/CriteriaEvalResultDropdown.tsx b/teachertool/src/components/CriteriaEvalResultDropdown.tsx index fbabb262bc2a..8c6be744aeb8 100644 --- a/teachertool/src/components/CriteriaEvalResultDropdown.tsx +++ b/teachertool/src/components/CriteriaEvalResultDropdown.tsx @@ -56,7 +56,7 @@ export const CriteriaEvalResultDropdown: React.FC = ({ selectedId={selectedResult} className={classList("rounded", selectedResult)} items={dropdownItems} - onItemSelected={id => setEvalResultOutcome(criteriaId, itemIdToCriteriaResult[id])} + onItemSelected={id => setEvalResultOutcome(criteriaId, itemIdToCriteriaResult[id], true)} /> ); }; diff --git a/teachertool/src/components/CriteriaInstanceDisplay.tsx b/teachertool/src/components/CriteriaInstanceDisplay.tsx index 0cf977b44726..d5b2be707794 100644 --- a/teachertool/src/components/CriteriaInstanceDisplay.tsx +++ b/teachertool/src/components/CriteriaInstanceDisplay.tsx @@ -9,7 +9,7 @@ import { useContext, useEffect, useMemo, useState } from "react"; import { Input } from "react-common/components/controls/Input"; import { Button } from "react-common/components/controls/Button"; import { AppStateContext } from "../state/appStateContext"; -import { Strings } from "../constants"; +import { Strings, Ticks } from "../constants"; import { showModal } from "../transforms/showModal"; import { BlockPickerOptions } from "../types/modalOptions"; import { validateParameterValue } from "../utils/validateParameterValue"; @@ -91,6 +91,7 @@ interface BlockData { const BlockInputSegment: React.FC = ({ instance, param }) => { const { state: teacherTool } = useContext(AppStateContext); function handleClick() { + pxt.tickEvent(Ticks.BlockPickerOpened, { criteriaCatalogId: instance.catalogCriteriaId }); showModal({ modal: "block-picker", criteriaInstanceId: instance.instanceId, diff --git a/teachertool/src/components/CriteriaResultEntry.tsx b/teachertool/src/components/CriteriaResultEntry.tsx index 8aef5ccb998a..ccb160c6dc0b 100644 --- a/teachertool/src/components/CriteriaResultEntry.tsx +++ b/teachertool/src/components/CriteriaResultEntry.tsx @@ -17,7 +17,7 @@ import { removeCriteriaFromChecklist } from "../transforms/removeCriteriaFromChe import { Button } from "react-common/components/controls/Button"; import { setEvalResult } from "../transforms/setEvalResult"; import { showToast } from "../transforms/showToast"; -import { makeToast } from "../utils"; +import { getChecklistHash, getObfuscatedProjectId, makeToast } from "../utils"; interface CriteriaResultNotesProps { criteriaId: string; @@ -69,7 +69,11 @@ const CriteriaResultError: React.FC = ({ criteriaInsta leftIcon="fas fa-times-circle" title={Strings.Dismiss} onClick={() => - setEvalResult(criteriaInstanceId, { result: EvaluationStatus.NotStarted, error: undefined }) + setEvalResult(criteriaInstanceId, { + result: EvaluationStatus.NotStarted, + resultIsManual: false, + error: undefined, + }) } />
@@ -80,7 +84,12 @@ const CriteriaResultToolbarTray: React.FC<{ criteriaId: string }> = ({ criteriaI const { state: teacherTool } = useContext(AppStateContext); async function handleEvaluateClickedAsync() { - pxt.tickEvent(Ticks.Evaluate); + const criteriaInstance = getCriteriaInstanceWithId(teacherTool, criteriaId); + pxt.tickEvent(Ticks.SingleEvaluate, { + catalogCriteriaId: criteriaInstance?.catalogCriteriaId ?? "", + checklistHash: getChecklistHash(teacherTool.checklist), + projectId: getObfuscatedProjectId(teacherTool.projectMetadata?.id), + }); const success = await runSingleEvaluateAsync(criteriaId, true); if (success) { diff --git a/teachertool/src/components/EvalResultDisplay.tsx b/teachertool/src/components/EvalResultDisplay.tsx index 84bd2cf4077a..7d2b187f00c9 100644 --- a/teachertool/src/components/EvalResultDisplay.tsx +++ b/teachertool/src/components/EvalResultDisplay.tsx @@ -5,7 +5,7 @@ import { useReactToPrint } from "react-to-print"; import { AppStateContext } from "../state/appStateContext"; import { CriteriaResultEntry } from "./CriteriaResultEntry"; import { QRCodeSVG } from "qrcode.react"; -import { getProjectLink } from "../utils"; +import { getChecklistHash, getObfuscatedProjectId, getProjectLink } from "../utils"; import { classList } from "react-common/components/util"; import { AddCriteriaButton } from "./AddCriteriaButton"; import { DebouncedInput } from "./DebouncedInput"; @@ -30,7 +30,15 @@ const ResultsHeader: React.FC = ({ printRef }) => { const [checklistNameInputRef, setChecklistNameInputRef] = useState(); const handleEvaluateClickedAsync = async () => { - pxt.tickEvent(Ticks.Evaluate); + pxt.tickEvent(Ticks.BulkEvaluate, { + fromUserInteraction: true + "", + runOnLoad: false + "", + criteriaCount: checklist.criteria.length, + catalogCriteriaIds: JSON.stringify(checklist.criteria.map(c => c.catalogCriteriaId)), + checklistHash: getChecklistHash(checklist), + projectId: getObfuscatedProjectId(projectMetadata?.id), + }); + await runEvaluateAsync(true); }; diff --git a/teachertool/src/components/HomeScreen.tsx b/teachertool/src/components/HomeScreen.tsx index 6d345ebcfc66..2a4aeb3c808b 100644 --- a/teachertool/src/components/HomeScreen.tsx +++ b/teachertool/src/components/HomeScreen.tsx @@ -88,7 +88,6 @@ interface ChecklistResourceCardProps { const ChecklistResourceCard: React.FC = ({ cardTitle, imageUrl, checklistUrl }) => { const onCardClickedAsync = async () => { - pxt.tickEvent(Ticks.LoadChecklist, { checklistUrl }); await loadChecklistAsync(checklistUrl); }; return ( @@ -148,7 +147,7 @@ const GetStarted: React.FC = () => { }; const onImportChecklistClicked = () => { - pxt.tickEvent(Ticks.ImportChecklist); + pxt.tickEvent(Ticks.ImportChecklistOpen); showModal({ modal: "import-checklist" } as ImportChecklistOptions); }; diff --git a/teachertool/src/components/ImportChecklistModal.tsx b/teachertool/src/components/ImportChecklistModal.tsx index ff8ee21c1b76..f94a0d098fa1 100644 --- a/teachertool/src/components/ImportChecklistModal.tsx +++ b/teachertool/src/components/ImportChecklistModal.tsx @@ -4,7 +4,7 @@ import { Modal } from "react-common/components/controls/Modal"; import { hideModal } from "../transforms/hideModal"; import { getChecklistFromFileAsync } from "../transforms/getChecklistFromFileAsync"; import { DragAndDropFileSurface } from "./DragAndDropFileSurface"; -import { Strings } from "../constants"; +import { Strings, Ticks } from "../constants"; import css from "./styling/ImportChecklistModal.module.scss"; import { replaceActiveChecklistAsync } from "../transforms/replaceActiveChecklistAsync"; @@ -14,7 +14,8 @@ export const ImportChecklistModal: React.FC = () => { const { state: teacherTool } = useContext(AppStateContext); const [errorMessage, setErrorMessage] = useState(undefined); - function closeModal() { + function closeModal(hasImported: boolean) { + pxt.tickEvent(Ticks.ImportChecklistClose, { hasImported: hasImported + "" }); setErrorMessage(undefined); hideModal(); } @@ -22,16 +23,22 @@ export const ImportChecklistModal: React.FC = () => { async function handleFileDroppedAsync(file: File) { const parsedChecklist = await getChecklistFromFileAsync(file, false /* allow partial */); if (!parsedChecklist) { + pxt.tickEvent(Ticks.ImportChecklistInvalidFile); setErrorMessage(Strings.InvalidChecklistFile); } else { + pxt.tickEvent(Ticks.ImportChecklistSuccess); setErrorMessage(undefined); - closeModal(); + closeModal(true); replaceActiveChecklistAsync(parsedChecklist); } } return teacherTool.modalOptions?.modal === "import-checklist" ? ( - + closeModal(false)} + className={css["import-checklist-modal"]} + >
diff --git a/teachertool/src/components/ShareLinkInput.tsx b/teachertool/src/components/ShareLinkInput.tsx index 4db4d70e2802..d63458e44fca 100644 --- a/teachertool/src/components/ShareLinkInput.tsx +++ b/teachertool/src/components/ShareLinkInput.tsx @@ -5,6 +5,9 @@ import { AppStateContext } from "../state/appStateContext"; import { classList } from "react-common/components/util"; import { Input } from "react-common/components/controls/Input"; import { loadProjectMetadataAsync } from "../transforms/loadProjectMetadataAsync"; +import { Strings, Ticks } from "../constants"; +import { getChecklistHash, makeToast } from "../utils"; +import { showToast } from "../transforms/showToast"; interface IProps {} @@ -26,7 +29,14 @@ export const ShareLinkInput: React.FC = () => { const onEnterKey = useCallback(() => { const shareId = pxt.Cloud.parseScriptId(text); - if (!!shareId && !(shareId === projectMetadata?.shortid || shareId === projectMetadata?.persistId)) { + if (!shareId) { + pxt.tickEvent(Ticks.LoadProjectInvalid); + showToast(makeToast("error", Strings.InvalidShareLink)); + return; + } + + if (shareId !== projectMetadata?.shortid && shareId !== projectMetadata?.persistId) { + pxt.tickEvent(Ticks.LoadProjectFromInput, { checklistHash: getChecklistHash(teacherTool.checklist) }); loadProjectMetadataAsync(text, shareId); } }, [text, projectMetadata?.shortid, projectMetadata?.persistId]); diff --git a/teachertool/src/constants.ts b/teachertool/src/constants.ts index 262dc5831d5c..a62f5b5ab314 100644 --- a/teachertool/src/constants.ts +++ b/teachertool/src/constants.ts @@ -58,6 +58,7 @@ export namespace Strings { export const BelowMin = lf("Below minimum value"); export const ExceedsMax = lf("Exceeds maximum value"); export const InvalidValue = lf("Invalid value"); + export const InvalidShareLink = lf("Invalid share link"); } export namespace Ticks { @@ -67,10 +68,14 @@ export namespace Ticks { export const OrgLink = "teachertool.orglink"; export const Error = "teachertool.error"; export const NewChecklist = "teachertool.newchecklist"; - export const ImportChecklist = "teachertool.importchecklist"; + export const ImportChecklistOpen = "teachertool.importchecklist.open"; + export const ImportChecklistInvalidFile = "teachertool.importchecklist.invalidfile"; + export const ImportChecklistSuccess = "teachertool.importchecklist.success"; + export const ImportChecklistClose = "teachertool.importchecklist.close"; export const ExportChecklist = "teachertool.exportchecklist"; - export const LoadChecklist = "teachertool.loadchecklist"; - export const Evaluate = "teachertool.evaluate"; + export const LoadChecklistFromUrl = "teachertool.loadchecklistfromurl"; + export const SingleEvaluate = "teachertool.singleevaluate"; + export const BulkEvaluate = "teachertool.bulkevaluate"; export const RunOnLoad = "teachertool.runonload"; export const AddCriteria = "teachertool.addcriteria"; export const RemoveCriteria = "teachertool.removecriteria"; @@ -83,6 +88,13 @@ export namespace Ticks { export const UnhandledEvalError = "teachertool.evaluateerror"; export const FeedbackForm = "teachertool.feedbackform"; export const ParamErrorMissingMessage = "teachertool.paramerrormissingmessage"; + export const SetEvalResultOutcome = "teachertool.setevalresultoutcome"; + export const SetEvalResultNotes = "teachertool.setevalresultnotes"; + export const LoadProjectFromInput = "teachertool.loadproject.frominput"; + export const LoadProjectFromUrl = "teachertool.loadproject.fromurl"; + export const LoadProjectInvalid = "teachertool.loadproject.invalid"; + export const BlockPickerBlockSelected = "teachertool.blockpicker.blockselected"; + export const BlockPickerOpened = "teachertool.blockpicker.opened"; } namespace Misc { diff --git a/teachertool/src/services/makecodeEditorService.ts b/teachertool/src/services/makecodeEditorService.ts index be6302717c5e..9ffec604b988 100644 --- a/teachertool/src/services/makecodeEditorService.ts +++ b/teachertool/src/services/makecodeEditorService.ts @@ -6,6 +6,8 @@ import { EditorDriver } from "pxtservices/editorDriver"; import { loadToolboxCategoriesAsync } from "../transforms/loadToolboxCategoriesAsync"; import { stateAndDispatch } from "../state"; import { runEvaluateAsync } from "../transforms/runEvaluateAsync"; +import { Ticks } from "../constants"; +import { getChecklistHash, getObfuscatedProjectId } from "../utils"; let driver: EditorDriver | undefined; let highContrast: boolean = false; @@ -36,6 +38,15 @@ export function setEditorRef(ref: HTMLIFrameElement | undefined, forceReload: () const { runOnLoad, projectMetadata } = state; if (runOnLoad && !!projectMetadata) { + pxt.tickEvent(Ticks.BulkEvaluate, { + fromUserInteraction: true + "", + runOnLoad: true + "", + criteriaCount: state.checklist.criteria.length, + catalogCriteriaIds: JSON.stringify(state.checklist.criteria.map(c => c.catalogCriteriaId)), + checklistHash: getChecklistHash(state.checklist), + projectId: getObfuscatedProjectId(state.projectMetadata?.id), + }); + runEvaluateAsync(true); // cause a switch to checklist tab on run } diff --git a/teachertool/src/state/helpers.ts b/teachertool/src/state/helpers.ts index cd62e7650409..14951e78bdb9 100644 --- a/teachertool/src/state/helpers.ts +++ b/teachertool/src/state/helpers.ts @@ -68,7 +68,7 @@ export function isProjectLoaded(state: AppState): boolean { } export function isChecklistLoaded(state: AppState): boolean { - return !!(state.checklist.criteria.length || state.checklist.name); + return !!state.checklist.criteria.length; } export function getSafeProjectName(state: AppState): string | undefined { diff --git a/teachertool/src/transforms/loadChecklistAsync.ts b/teachertool/src/transforms/loadChecklistAsync.ts index a0e22fb0c363..a4b1d24fcf7e 100644 --- a/teachertool/src/transforms/loadChecklistAsync.ts +++ b/teachertool/src/transforms/loadChecklistAsync.ts @@ -1,25 +1,30 @@ -import { Strings } from "../constants"; +import { Strings, Ticks } from "../constants"; import { fetchJsonDocAsync } from "../services/backendRequests"; +import { logError } from "../services/loggingService"; import { verifyChecklistIntegrity } from "../state/helpers"; import { Checklist } from "../types/checklist"; -import { makeToast } from "../utils"; +import { ErrorCode } from "../types/errorCode"; +import { getChecklistHash, makeToast } from "../utils"; import { replaceActiveChecklistAsync } from "./replaceActiveChecklistAsync"; import { showToast } from "./showToast"; export async function loadChecklistAsync(checklistUrl: string) { - const json = await fetchJsonDocAsync(checklistUrl); + const checklist = await fetchJsonDocAsync(checklistUrl); - if (!json) { + if (!checklist) { showToast(makeToast("error", Strings.ErrorLoadingChecklistMsg)); return; } - const { valid } = verifyChecklistIntegrity(json); + const { valid } = verifyChecklistIntegrity(checklist); if (!valid) { + logError(ErrorCode.invalidPremadeChecklist, { checklistUrl }); showToast(makeToast("error", Strings.ErrorLoadingChecklistMsg)); return; + } else { + pxt.tickEvent(Ticks.LoadChecklistFromUrl, { checklistUrl, checklistHash: getChecklistHash(checklist) }); } - await replaceActiveChecklistAsync(json); + await replaceActiveChecklistAsync(checklist); } diff --git a/teachertool/src/transforms/mergeEvalResult.ts b/teachertool/src/transforms/mergeEvalResult.ts index 26427b4adce5..82ba038e1f0f 100644 --- a/teachertool/src/transforms/mergeEvalResult.ts +++ b/teachertool/src/transforms/mergeEvalResult.ts @@ -4,7 +4,12 @@ import { setEvalResult } from "./setEvalResult"; import { setUserFeedback } from "./setUserFeedback"; // This will set the outcome and notes for a given criteria instance id, but if the provided value is undefined, it will not change that value. -export function mergeEvalResult(criteriaInstanceId: string, outcome?: EvaluationStatus, notes?: string) { +export function mergeEvalResult( + criteriaInstanceId: string, + isManual: boolean, + outcome?: EvaluationStatus, + notes?: string +) { const { state: teacherTool, dispatch } = stateAndDispatch(); const newCriteriaEvalResult = { ...teacherTool.evalResults[criteriaInstanceId] }; @@ -14,6 +19,7 @@ export function mergeEvalResult(criteriaInstanceId: string, outcome?: Evaluation if (outcome !== undefined) { newCriteriaEvalResult.result = outcome; + newCriteriaEvalResult.resultIsManual = isManual; } if (notes !== undefined) { if (newCriteriaEvalResult.notes !== notes) { diff --git a/teachertool/src/transforms/runSingleEvaluateAsync.ts b/teachertool/src/transforms/runSingleEvaluateAsync.ts index 755692830993..cdc924e62672 100644 --- a/teachertool/src/transforms/runSingleEvaluateAsync.ts +++ b/teachertool/src/transforms/runSingleEvaluateAsync.ts @@ -121,7 +121,7 @@ export async function runSingleEvaluateAsync(criteriaInstanceId: string, fromUse return resolve(true); } - setEvalResultOutcome(criteriaInstance.instanceId, EvaluationStatus.InProgress); + setEvalResultOutcome(criteriaInstance.instanceId, EvaluationStatus.InProgress, false); const loadedValidatorPlans = teacherTool.validatorPlans; if (!loadedValidatorPlans) { @@ -154,11 +154,12 @@ export async function runSingleEvaluateAsync(criteriaInstanceId: string, fromUse ? EvaluationStatus.Pass : EvaluationStatus.Fail; - mergeEvalResult(criteriaInstance.instanceId, result, planResult.notes); + mergeEvalResult(criteriaInstance.instanceId, false, result, planResult.notes); return resolve(true); // evaluation completed successfully, so return true (regardless of pass/fail) } else { setEvalResult(criteriaInstance.instanceId, { result: EvaluationStatus.NotStarted, + resultIsManual: false, error: planResult?.executionErrorMsg ?? Strings.UnexpectedError, }); setUserFeedback(criteriaInstanceId, undefined); @@ -174,6 +175,7 @@ export async function runSingleEvaluateAsync(criteriaInstanceId: string, fromUse setUserFeedback(criteriaInstanceId, undefined); setEvalResult(criteriaInstance.instanceId, { result: EvaluationStatus.NotStarted, + resultIsManual: false, error: Strings.UnexpectedError, }); return resolve(false); diff --git a/teachertool/src/transforms/setEvalResult.ts b/teachertool/src/transforms/setEvalResult.ts index 1f05e92f2709..db915743c7ee 100644 --- a/teachertool/src/transforms/setEvalResult.ts +++ b/teachertool/src/transforms/setEvalResult.ts @@ -1,8 +1,40 @@ +import { Ticks } from "../constants"; +import { logError } from "../services/loggingService"; import { stateAndDispatch } from "../state"; import * as Actions from "../state/actions"; -import { CriteriaResult } from "../types/criteria"; +import { getCriteriaInstanceWithId } from "../state/helpers"; +import { CriteriaResult, EvaluationStatus } from "../types/criteria"; +import { ErrorCode } from "../types/errorCode"; + +// Send tick events related to changes happening in the criteria result. +function reportChanges(criteriaId: string, result: CriteriaResult) { + const { state: teacherTool } = stateAndDispatch(); + + const previousResult = teacherTool.evalResults[criteriaId]; + const criteriaInstance = getCriteriaInstanceWithId(teacherTool, criteriaId); + + if (previousResult.result != result.result) { + pxt.tickEvent(Ticks.SetEvalResultOutcome, { + catalogCriteriaId: criteriaInstance?.catalogCriteriaId ?? "", + newValue: EvaluationStatus[result.result], + oldValue: previousResult?.result ? EvaluationStatus[previousResult.result] : "", + newValueIsManual: result.resultIsManual + "", + oldValueIsManual: previousResult?.resultIsManual + "", + }); + } + + if (previousResult.notes != result.notes) { + // Setting notes is debounced so this isn't too noisy. + pxt.tickEvent(Ticks.SetEvalResultNotes, { + catalogCriteriaId: criteriaInstance?.catalogCriteriaId ?? "", + newLength: result.notes?.length ?? 0, + oldLength: previousResult?.notes?.length ?? 0, + }); + } +} export function setEvalResult(criteriaId: string, result: CriteriaResult) { const { dispatch } = stateAndDispatch(); + reportChanges(criteriaId, result); dispatch(Actions.setEvalResult(criteriaId, result)); } diff --git a/teachertool/src/transforms/setEvalResultOutcome.ts b/teachertool/src/transforms/setEvalResultOutcome.ts index f4e270664ad6..922ca68194ca 100644 --- a/teachertool/src/transforms/setEvalResultOutcome.ts +++ b/teachertool/src/transforms/setEvalResultOutcome.ts @@ -3,12 +3,13 @@ import { EvaluationStatus } from "../types/criteria"; import { setEvalResult } from "./setEvalResult"; // This will set the outcome for a given criteria instance id. If result is undefined, it will clear it. -export function setEvalResultOutcome(criteriaId: string, result: EvaluationStatus) { - const { state: teacherTool, dispatch } = stateAndDispatch(); +export function setEvalResultOutcome(criteriaId: string, result: EvaluationStatus, isManual: boolean) { + const { state: teacherTool } = stateAndDispatch(); const newCriteriaEvalResult = { ...teacherTool.evalResults[criteriaId], result, + resultIsManual: isManual, }; setEvalResult(criteriaId, newCriteriaEvalResult); diff --git a/teachertool/src/transforms/setParameterValue.ts b/teachertool/src/transforms/setParameterValue.ts index 05aa572f3a6c..fdba4bc3c48f 100644 --- a/teachertool/src/transforms/setParameterValue.ts +++ b/teachertool/src/transforms/setParameterValue.ts @@ -36,5 +36,5 @@ export function setParameterValue(instanceId: string, paramName: string, newValu const newChecklist = { ...teacherTool.checklist, criteria: newInstanceSet }; setChecklist(newChecklist); - setEvalResultOutcome(instanceId, EvaluationStatus.NotStarted); + setEvalResultOutcome(instanceId, EvaluationStatus.NotStarted, false); } diff --git a/teachertool/src/transforms/setRunOnLoad.ts b/teachertool/src/transforms/setRunOnLoad.ts index f91efee2fc94..3d1416de82d8 100644 --- a/teachertool/src/transforms/setRunOnLoad.ts +++ b/teachertool/src/transforms/setRunOnLoad.ts @@ -1,7 +1,6 @@ import { stateAndDispatch } from "../state"; import * as Actions from "../state/actions"; import * as Storage from "../services/storageService"; -import { runEvaluateAsync } from "./runEvaluateAsync"; export function setRunOnLoad(runOnLoad: boolean) { const { dispatch } = stateAndDispatch(); diff --git a/teachertool/src/types/criteria.ts b/teachertool/src/types/criteria.ts index 2d8ab865d52e..74ad2d66a76f 100644 --- a/teachertool/src/types/criteria.ts +++ b/teachertool/src/types/criteria.ts @@ -40,6 +40,7 @@ export enum EvaluationStatus { export interface CriteriaResult { result: EvaluationStatus; + resultIsManual?: boolean; notes?: string; error?: string; } diff --git a/teachertool/src/types/criteriaParameters.ts b/teachertool/src/types/criteriaParameters.ts index 4c78e05096cd..cebc60b7f2a6 100644 --- a/teachertool/src/types/criteriaParameters.ts +++ b/teachertool/src/types/criteriaParameters.ts @@ -6,29 +6,29 @@ export type CriteriaParameterBase = { type: CriteriaParameterType; default: string | undefined; paths: string[]; // The json path(s) to update with the parameter value in the catalog criteria. -} +}; export type StringParameterBase = CriteriaParameterBase & { maxCharacters?: number; -} +}; export type StringParameter = StringParameterBase & { type: "string"; -} +}; export type LongStringParameter = StringParameterBase & { type: "longString"; -} +}; export type NumberParameter = CriteriaParameterBase & { type: "number"; min?: number; max?: number; -} +}; export type BlockParameter = CriteriaParameterBase & { type: "block"; -} +}; /** * System parameters are fields that can change for a criteria but which are not set directly by the user. @@ -37,9 +37,14 @@ export type BlockParameter = CriteriaParameterBase & { export type SystemParameter = CriteriaParameterBase & { type: "system"; key?: string; -} - -export type CriteriaParameter = StringParameter | LongStringParameter | NumberParameter | BlockParameter | SystemParameter; +}; + +export type CriteriaParameter = + | StringParameter + | LongStringParameter + | NumberParameter + | BlockParameter + | SystemParameter; export interface CriteriaParameterValidationResult { valid: boolean; diff --git a/teachertool/src/types/errorCode.ts b/teachertool/src/types/errorCode.ts index 8f0043082d83..d0fe8330166e 100644 --- a/teachertool/src/types/errorCode.ts +++ b/teachertool/src/types/errorCode.ts @@ -34,4 +34,5 @@ export enum ErrorCode { authCheckFailed = "authCheckFailed", unrecognizedParameterType = "unrecognizedParameterType", invalidParameterValue = "invalidParameterValue", + invalidPremadeChecklist = "invalidPremadeChecklist", } diff --git a/teachertool/src/utils/index.ts b/teachertool/src/utils/index.ts index a8a35c78b165..e38300889952 100644 --- a/teachertool/src/utils/index.ts +++ b/teachertool/src/utils/index.ts @@ -80,3 +80,14 @@ export function getReadableBlockString(name: string) { return pxt.Util.camelCaseToLowercaseWithSpaces(name); } } + +export function getChecklistHash(checklist: Checklist): string { + // We only hash the criteria (not the name), since the name doesn't really matter in our scenarios, + // and it could be translated, etc for built-in checklists. + return checklist.criteria.length == 0 ? "empty" : pxt.Util.sha256(JSON.stringify(checklist.criteria)); +} + +export function getObfuscatedProjectId(projectId: string | undefined): string { + // Just to err on the safe side for privacy, don't log the whole share id. + return !projectId || projectId?.length <= 5 ? "" : "..." + projectId.slice(-5); +}