diff --git a/pxtblocks/code-validation/runValidatorPlanAsync.ts b/pxtblocks/code-validation/runValidatorPlanAsync.ts index d80ed7e234ec..d063c411565c 100644 --- a/pxtblocks/code-validation/runValidatorPlanAsync.ts +++ b/pxtblocks/code-validation/runValidatorPlanAsync.ts @@ -1,11 +1,14 @@ namespace pxt.blocks { + + const maxConcurrentChecks = 4; + export async function runValidatorPlanAsync(usedBlocks: Blockly.Block[], plan: ValidatorPlan): Promise { // Each plan can have multiple checks it needs to run. // Run all of them in parallel, and then check if the number of successes is greater than the specified threshold. // TBD if it's faster to run in parallel without short-circuiting once the threshold is reached, or if it's faster to run sequentially and short-circuit. const startTime = Date.now(); - const checkRuns = pxt.Util.promisePoolAsync(4, plan.checks, async (check: ValidatorCheckBase): Promise => { + const checkRuns = pxt.Util.promisePoolAsync(maxConcurrentChecks, plan.checks, async (check: ValidatorCheckBase): Promise => { switch (check.validator) { case "blocksExist": return runBlocksExistValidation(usedBlocks, check as BlocksExistValidatorCheck); @@ -18,14 +21,15 @@ namespace pxt.blocks { const results = await checkRuns; const successCount = results.filter((r) => r).length; + const passed = successCount >= plan.threshold; pxt.tickEvent("validation.evaluation_complete", { plan: plan.name, durationMs: Date.now() - startTime, - passed: `${successCount >= plan.threshold}`, + passed: `${passed}`, }); - return successCount >= plan.threshold; + return passed; } function runBlocksExistValidation(usedBlocks: Blockly.Block[], inputs: BlocksExistValidatorCheck): boolean { diff --git a/teachertool/src/App.tsx b/teachertool/src/App.tsx index e3e212b5ed60..fde995629a32 100644 --- a/teachertool/src/App.tsx +++ b/teachertool/src/App.tsx @@ -15,6 +15,7 @@ import { CatalogModal } from "./components/CatalogModal"; import { postNotification } from "./transforms/postNotification"; import { loadCatalogAsync } from "./transforms/loadCatalogAsync"; import { loadValidatorPlansAsync } from "./transforms/loadValidatorPlansAsync"; +import { tryLoadLastActiveRubricAsync } from "./transforms/tryLoadLastActiveRubricAsync"; export const App = () => { const { state, dispatch } = useContext(AppStateContext); @@ -34,6 +35,8 @@ export const App = () => { await loadCatalogAsync(); await loadValidatorPlansAsync(); + await tryLoadLastActiveRubricAsync(); + // TODO: Remove this. Delay app init to expose any startup race conditions. setTimeout(() => { // Test notification diff --git a/teachertool/src/components/ActiveRubricDisplay.tsx b/teachertool/src/components/ActiveRubricDisplay.tsx index 6ca897c6c70d..fbf3787c49c5 100644 --- a/teachertool/src/components/ActiveRubricDisplay.tsx +++ b/teachertool/src/components/ActiveRubricDisplay.tsx @@ -1,21 +1,30 @@ /// -import { useContext } from "react"; +import { useContext, useState } from "react"; import { AppStateContext } from "../state/appStateContext"; import { getCatalogCriteriaWithId } from "../state/helpers"; import { Button } from "react-common/components/controls/Button"; import { removeCriteriaFromRubric } from "../transforms/removeCriteriaFromRubric"; import { showCatalogModal } from "../transforms/showCatalogModal"; +import { setRubricName } from "../transforms/setRubricName"; +import { DebouncedInput } from "./DebouncedInput"; interface IProps {} export const ActiveRubricDisplay: React.FC = ({}) => { - const { state: teacherTool, dispatch } = useContext(AppStateContext); + const { state: teacherTool } = useContext(AppStateContext); return (
-

{lf("Rubric")}

- {teacherTool.selectedCriteria?.map(criteriaInstance => { + + {teacherTool.rubric.criteria?.map(criteriaInstance => { if (!criteriaInstance) return null; const catalogCriteria = getCatalogCriteriaWithId(criteriaInstance.catalogCriteriaId); diff --git a/teachertool/src/components/DebouncedInput.tsx b/teachertool/src/components/DebouncedInput.tsx new file mode 100644 index 000000000000..d9bfd5a1d79e --- /dev/null +++ b/teachertool/src/components/DebouncedInput.tsx @@ -0,0 +1,43 @@ +import { useEffect, useRef } from "react"; +import { Input, InputProps } from "react-common/components/controls/Input"; + +export interface DebouncedInputProps extends InputProps { + intervalMs?: number; // Default 500 ms +} + +// This functions like the React Common Input, but debounces onChange calls, +// so if onChange is called multiple times in quick succession, it will only +// be executed once after a pause of the specified `interval` in milliseconds. +export const DebouncedInput: React.FC = ({ intervalMs = 500, ...props }) => { + const timerId = useRef(undefined); + const latestValue = useRef(""); + + const sendChange = () => { + if (props.onChange) { + props.onChange(latestValue.current); + } + }; + + // If the timer is pending and the component unmounts, + // clear the timer and fire the onChange event immediately. + useEffect(() => { + return () => { + if (timerId.current) { + clearTimeout(timerId.current); + sendChange(); + } + }; + }, []); + + const onChangeDebounce = (newValue: string) => { + latestValue.current = newValue; + + if (timerId.current) { + clearTimeout(timerId.current); + } + + timerId.current = setTimeout(sendChange, intervalMs); + }; + + return ; +}; diff --git a/teachertool/src/components/EvalResultDisplay.tsx b/teachertool/src/components/EvalResultDisplay.tsx index b8a6127aef2c..d66fdab0999c 100644 --- a/teachertool/src/components/EvalResultDisplay.tsx +++ b/teachertool/src/components/EvalResultDisplay.tsx @@ -11,7 +11,7 @@ export const EvalResultDisplay: React.FC = ({}) => { const { state: teacherTool } = useContext(AppStateContext); function getTemplateStringFromCriteriaInstanceId(instanceId: string): string { - const catalogCriteriaId = teacherTool.selectedCriteria?.find( + const catalogCriteriaId = teacherTool.rubric.criteria?.find( criteria => criteria.instanceId === instanceId )?.catalogCriteriaId; if (!catalogCriteriaId) return ""; diff --git a/teachertool/src/services/indexedDbService.ts b/teachertool/src/services/indexedDbService.ts new file mode 100644 index 000000000000..3a933ae0d634 --- /dev/null +++ b/teachertool/src/services/indexedDbService.ts @@ -0,0 +1,128 @@ +import { openDB, IDBPDatabase } from "idb"; +import { ErrorCode } from "../types/errorCode"; +import { logError } from "./loggingService"; +import { Rubric } from "../types/rubric"; + +const teacherToolDbName = "makecode-project-insights"; +const dbVersion = 1; +const rubricsStoreName = "rubrics"; +const metadataStoreName = "metadata"; +const metadataKeys = { + lastActiveRubricKey: "lastActiveRubricName", +}; + +type MetadataEntry = { key: string; value: any }; + +class TeacherToolDb { + db: IDBPDatabase | undefined; + + public async initializeAsync() { + if (this.db) return; + this.db = await openDB(teacherToolDbName, dbVersion, { + upgrade(db) { + db.createObjectStore(rubricsStoreName, { keyPath: "name" }); + db.createObjectStore(metadataStoreName, { keyPath: "key" }); + }, + }); + } + + private async getAsync(storeName: string, key: string): Promise { + if (!this.db) { + throw new Error("IndexedDb not initialized."); + } + + try { + return await this.db.get(storeName, key); + } catch (e) { + // Not recording key, as it could contain user-input with sensitive information. + logError(ErrorCode.unableToGetIndexedDbRecord, e); + } + } + + private async setAsync(storeName: string, value: T): Promise { + if (!this.db) { + throw new Error("IndexedDb not initialized."); + } + + try { + await this.db.put(storeName, value); + } catch (e) { + // Not recording key, as it could contain user-input with sensitive information. + logError(ErrorCode.unableToSetIndexedDbRecord, e); + } + } + + private async deleteAsync(storeName: string, key: string): Promise { + if (!this.db) { + throw new Error("IndexedDb not initialized."); + } + try { + await this.db.delete(storeName, key); + } catch (e) { + // Not recording key, as it could contain user-input with sensitive information. + logError(ErrorCode.unableToDeleteIndexedDbRecord, e); + } + } + + private async getMetadataEntryAsync(key: string): Promise { + return this.getAsync(metadataStoreName, key); + } + + private async setMetadataEntryAsync(key: string, value: any): Promise { + return this.setAsync(metadataStoreName, { key, value }); + } + + private async deleteMetadataEntryAsync(key: string): Promise { + return this.deleteAsync(metadataStoreName, key); + } + + public async getLastActiveRubricNameAsync(): Promise { + const metadataEntry = await this.getMetadataEntryAsync(metadataKeys.lastActiveRubricKey); + return metadataEntry?.value; + } + + public saveLastActiveRubricNameAsync(name: string): Promise { + return this.setMetadataEntryAsync(metadataKeys.lastActiveRubricKey, name); + } + + public getRubric(name: string): Promise { + return this.getAsync(rubricsStoreName, name); + } + + public saveRubric(rubric: Rubric): Promise { + return this.setAsync(rubricsStoreName, rubric); + } + + public deleteRubric(name: string): Promise { + return this.deleteAsync(rubricsStoreName, name); + } +} + +const getDb = (async () => { + const db = new TeacherToolDb(); + await db.initializeAsync(); + return db; +})(); + +export async function getLastActiveRubricAsync(): Promise { + const db = await getDb; + + let rubric: Rubric | undefined = undefined; + const name = await db.getLastActiveRubricNameAsync(); + if (name) { + rubric = await db.getRubric(name); + } + + return rubric; +} + +export async function saveRubricAsync(rubric: Rubric) { + const db = await getDb; + await db.saveRubric(rubric); + await db.saveLastActiveRubricNameAsync(rubric.name); +} + +export async function deleteRubricAsync(name: string) { + const db = await getDb; + await db.deleteRubric(name); +} diff --git a/teachertool/src/state/actions.ts b/teachertool/src/state/actions.ts index 6b16bd58f80d..32d23980d497 100644 --- a/teachertool/src/state/actions.ts +++ b/teachertool/src/state/actions.ts @@ -1,5 +1,6 @@ import { ModalType, NotificationWithId } from "../types"; -import { CatalogCriteria, CriteriaEvaluationResult, CriteriaInstance } from "../types/criteria"; +import { CatalogCriteria, CriteriaEvaluationResult } from "../types/criteria"; +import { Rubric } from "../types/rubric"; // Changes to app state are performed by dispatching actions to the reducer type ActionBase = { @@ -49,14 +50,9 @@ type SetCatalog = ActionBase & { catalog: CatalogCriteria[] | undefined; }; -type SetSelectedCriteria = ActionBase & { - type: "SET_SELECTED_CRITERIA"; - criteria: CriteriaInstance[]; -}; - -type RemoveCriteriaInstance = ActionBase & { - type: "REMOVE_CRITERIA_INSTANCE"; - instanceId: string; +type SetRubric = ActionBase & { + type: "SET_RUBRIC"; + rubric: Rubric; }; type ShowModal = ActionBase & { @@ -86,8 +82,7 @@ export type Action = | ClearAllEvalResults | SetTargetConfig | SetCatalog - | SetSelectedCriteria - | RemoveCriteriaInstance + | SetRubric | ShowModal | HideModal | SetValidatorPlans; @@ -135,14 +130,9 @@ const setCatalog = (catalog: CatalogCriteria[] | undefined): SetCatalog => ({ catalog, }); -const setSelectedCriteria = (criteria: CriteriaInstance[]): SetSelectedCriteria => ({ - type: "SET_SELECTED_CRITERIA", - criteria, -}); - -const removeCriteriaInstance = (instanceId: string): RemoveCriteriaInstance => ({ - type: "REMOVE_CRITERIA_INSTANCE", - instanceId, +const setRubric = (rubric: Rubric): SetRubric => ({ + type: "SET_RUBRIC", + rubric, }); const showModal = (modal: ModalType): ShowModal => ({ @@ -168,8 +158,7 @@ export { clearAllEvalResults, setTargetConfig, setCatalog, - setSelectedCriteria, - removeCriteriaInstance, + setRubric, showModal, hideModal, setValidatorPlans, diff --git a/teachertool/src/state/reducer.ts b/teachertool/src/state/reducer.ts index 99c91ec604ed..7dff79b9e5eb 100644 --- a/teachertool/src/state/reducer.ts +++ b/teachertool/src/state/reducer.ts @@ -1,5 +1,6 @@ import { AppState } from "./state"; import { Action } from "./actions"; +import { updateStoredRubricAsync } from "../transforms/updateStoredRubric"; // The reducer's job is to apply state changes by creating a copy of the existing state with the change applied. // The reducer must not create side effects. E.g. do not dispatch a state change from within the reducer. @@ -59,16 +60,11 @@ export default function reducer(state: AppState, action: Action): AppState { catalog: action.catalog, }; } - case "SET_SELECTED_CRITERIA": { + case "SET_RUBRIC": { + /*await*/ updateStoredRubricAsync(state.rubric, action.rubric); // fire and forget, we don't need to wait for this to finish. return { ...state, - selectedCriteria: [...action.criteria], - }; - } - case "REMOVE_CRITERIA_INSTANCE": { - return { - ...state, - selectedCriteria: state.selectedCriteria.filter(c => c.instanceId !== action.instanceId), + rubric: action.rubric, }; } case "SHOW_MODAL": { diff --git a/teachertool/src/state/state.ts b/teachertool/src/state/state.ts index c8fa919c8e3c..7180ad9c1c52 100644 --- a/teachertool/src/state/state.ts +++ b/teachertool/src/state/state.ts @@ -1,5 +1,6 @@ import { ModalType, Notifications } from "../types"; import { CatalogCriteria, CriteriaEvaluationResult, CriteriaInstance } from "../types/criteria"; +import { Rubric } from "../types/rubric"; export type AppState = { targetConfig?: pxt.TargetConfig; @@ -7,7 +8,7 @@ export type AppState = { evalResults: pxt.Map; // Criteria Instance Id -> Result projectMetadata: pxt.Cloud.JsonScript | undefined; catalog: CatalogCriteria[] | undefined; - selectedCriteria: CriteriaInstance[]; + rubric: Rubric; modal: ModalType | undefined; validatorPlans: pxt.blocks.ValidatorPlan[] | undefined; flags: { @@ -20,7 +21,7 @@ export const initialAppState: AppState = { evalResults: {}, projectMetadata: undefined, catalog: undefined, - selectedCriteria: [], + rubric: { name: "", criteria: [] }, modal: undefined, validatorPlans: undefined, flags: { diff --git a/teachertool/src/transforms/addCriteriaToRubric.ts b/teachertool/src/transforms/addCriteriaToRubric.ts index d0e2c0660d35..c262831564dd 100644 --- a/teachertool/src/transforms/addCriteriaToRubric.ts +++ b/teachertool/src/transforms/addCriteriaToRubric.ts @@ -10,7 +10,11 @@ export function addCriteriaToRubric(catalogCriteriaIds: string[]) { const { state: teacherTool, dispatch } = stateAndDispatch(); // Create instances for each of the catalog criteria. - const newSelectedCriteria = [...(teacherTool.selectedCriteria ?? [])]; + const newRubric = { + ...teacherTool.rubric, + criteria: [...(teacherTool.rubric.criteria ?? [])], + }; + for (const catalogCriteriaId of catalogCriteriaIds) { const catalogCriteria = getCatalogCriteriaWithId(catalogCriteriaId); if (!catalogCriteria) { @@ -37,10 +41,10 @@ export function addCriteriaToRubric(catalogCriteriaIds: string[]) { params, } as CriteriaInstance; - newSelectedCriteria.push(criteriaInstance); + newRubric.criteria.push(criteriaInstance); } - dispatch(Actions.setSelectedCriteria(newSelectedCriteria)); + dispatch(Actions.setRubric(newRubric)); pxt.tickEvent("teachertool.addcriteria", { ids: JSON.stringify(catalogCriteriaIds), diff --git a/teachertool/src/transforms/removeCriteriaFromRubric.ts b/teachertool/src/transforms/removeCriteriaFromRubric.ts index 6ab543e0d513..1dfb4f01ed60 100644 --- a/teachertool/src/transforms/removeCriteriaFromRubric.ts +++ b/teachertool/src/transforms/removeCriteriaFromRubric.ts @@ -4,11 +4,16 @@ import { logDebug } from "../services/loggingService"; import { CriteriaInstance } from "../types/criteria"; export function removeCriteriaFromRubric(instance: CriteriaInstance) { + const { state: teacherTool, dispatch } = stateAndDispatch(); + logDebug(`Removing criteria with id: ${instance.instanceId}`); - const { dispatch } = stateAndDispatch(); - dispatch(Actions.removeCriteriaInstance(instance.instanceId)); - pxt.tickEvent("teachertool.removecriteria", { - catalogCriteriaId: instance.catalogCriteriaId, - }); + const newRubric = { + ...teacherTool.rubric, + criteria: teacherTool.rubric.criteria.filter(c => c.instanceId !== instance.instanceId), + }; + + dispatch(Actions.setRubric(newRubric)); + + pxt.tickEvent("teachertool.removecriteria", { catalogCriteriaId: instance.catalogCriteriaId }); } diff --git a/teachertool/src/transforms/runEvaluateAsync.ts b/teachertool/src/transforms/runEvaluateAsync.ts index eaccfdc21d6a..15c39902018c 100644 --- a/teachertool/src/transforms/runEvaluateAsync.ts +++ b/teachertool/src/transforms/runEvaluateAsync.ts @@ -40,7 +40,7 @@ export async function runEvaluateAsync() { // EvalRequest promises will resolve to true if evaluation completed successfully (regarless of pass/fail). // They will only resolve to false if evaluation was unable to complete. - const evalRequests = teacherTool.selectedCriteria.map( + const evalRequests = teacherTool.rubric.criteria.map( criteriaInstance => new Promise(async resolve => { dispatch(Actions.setEvalResult(criteriaInstance.instanceId, CriteriaEvaluationResult.InProgress)); @@ -71,7 +71,7 @@ export async function runEvaluateAsync() { const results = await Promise.all(evalRequests); const errorCount = results.filter(r => !r).length; - if (errorCount === teacherTool.selectedCriteria.length) { + if (errorCount === teacherTool.rubric.criteria.length) { postNotification(makeNotification(lf("Unable to run evaluation"), 2000)); } else if (errorCount > 0) { postNotification(makeNotification(lf("Unable to evaluate some criteria"), 2000)); diff --git a/teachertool/src/transforms/setRubricName.ts b/teachertool/src/transforms/setRubricName.ts new file mode 100644 index 000000000000..6d43a74f4fe3 --- /dev/null +++ b/teachertool/src/transforms/setRubricName.ts @@ -0,0 +1,18 @@ +import { stateAndDispatch } from "../state"; +import * as Actions from "../state/actions"; + +export function setRubricName(name: string) { + const { state: teacherTool, dispatch } = stateAndDispatch(); + + const oldName = teacherTool.rubric.name; + + if (oldName === name) { + return; + } + + const newRubric = { + ...teacherTool.rubric, + name, + }; + dispatch(Actions.setRubric(newRubric)); +} diff --git a/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts b/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts new file mode 100644 index 000000000000..e8229cebd8d6 --- /dev/null +++ b/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts @@ -0,0 +1,56 @@ +import { stateAndDispatch } from "../state"; +import * as Actions from "../state/actions"; +import { getLastActiveRubricAsync } from "../services/indexedDbService"; +import { logDebug, logError } from "../services/loggingService"; +import { CriteriaInstance } from "../types/criteria"; +import { getCatalogCriteriaWithId } from "../state/helpers"; +import { ErrorCode } from "../types/errorCode"; +import { postNotification } from "./postNotification"; +import { makeNotification } from "../utils"; + +function validateCriteriaInstance(instance: CriteriaInstance) { + const catalogCriteria = getCatalogCriteriaWithId(instance.catalogCriteriaId); + + if (!catalogCriteria) { + logError( + ErrorCode.loadUnrecognizedCatalogId, + "Attempting to load criteria instance with unrecognized catalog id", + { catalogCriteriaId: instance.catalogCriteriaId } + ); + return false; + } + + for (const param of instance.params ?? []) { + if (!catalogCriteria?.parameters?.find(p => p.name === param.name)) { + logError( + ErrorCode.loadUnrecognizedParameter, + "Attempting to load criteria instance with unrecognized parameter", + { catalogCriteriaId: instance.catalogCriteriaId, paramName: param.name } + ); + return false; + } + } + + return true; +} + +export async function tryLoadLastActiveRubricAsync() { + const { dispatch } = stateAndDispatch(); + + const lastActiveRubric = await getLastActiveRubricAsync(); + + if (lastActiveRubric) { + logDebug(`Loading last active rubric '${lastActiveRubric.name}'...`); + + const initialCriteriaCount = lastActiveRubric.criteria.length; + lastActiveRubric.criteria = lastActiveRubric.criteria.filter(validateCriteriaInstance); + + if (lastActiveRubric.criteria.length !== initialCriteriaCount) { + postNotification(makeNotification(lf("Some criteria could not be loaded."), 2000)); + } + + dispatch(Actions.setRubric(lastActiveRubric)); + } else { + logDebug(`No last active rubric to load.`); + } +} diff --git a/teachertool/src/transforms/updateStoredRubric.ts b/teachertool/src/transforms/updateStoredRubric.ts new file mode 100644 index 000000000000..1533a9928820 --- /dev/null +++ b/teachertool/src/transforms/updateStoredRubric.ts @@ -0,0 +1,15 @@ +import { deleteRubricAsync, saveRubricAsync } from "../services/indexedDbService"; +import { Rubric } from "../types/rubric"; + +export async function updateStoredRubricAsync(oldRubric: Rubric | undefined, newRubric: Rubric | undefined) { + const renamed = oldRubric && newRubric && oldRubric?.name !== newRubric.name; + const deleted = oldRubric && !newRubric; + + if (newRubric) { + await saveRubricAsync(newRubric); + } + + if (renamed || deleted) { + await deleteRubricAsync(oldRubric.name); + } +} diff --git a/teachertool/src/types/errorCode.ts b/teachertool/src/types/errorCode.ts index c82dc7416c4f..3a2a17992b12 100644 --- a/teachertool/src/types/errorCode.ts +++ b/teachertool/src/types/errorCode.ts @@ -7,4 +7,9 @@ export enum ErrorCode { evalMissingCriteria = "evalMissingCriteria", evalMissingPlan = "evalMissingPlan", loadCollectionFileFailed = "loadCollectionFileFailed", + unableToGetIndexedDbRecord = "unableToGetIndexedDbRecord", + unableToSetIndexedDbRecord = "unableToSetIndexedDbRecord", + unableToDeleteIndexedDbRecord = "unableToDeleteIndexedDbRecord", + loadUnrecognizedCatalogId = "loadUnrecognizedCatalogId", + loadUnrecognizedParameter = "loadUnrecognizedParameter", } diff --git a/teachertool/src/types/rubric.ts b/teachertool/src/types/rubric.ts new file mode 100644 index 000000000000..073ca325c7d0 --- /dev/null +++ b/teachertool/src/types/rubric.ts @@ -0,0 +1,6 @@ +import { CriteriaInstance } from "./criteria"; + +export interface Rubric { + name: string; + criteria: CriteriaInstance[]; +}