From 6a63633e3f47bd554eac9bf336bf8b49bf9c94a8 Mon Sep 17 00:00:00 2001 From: Thomas Sparks Date: Tue, 30 Jan 2024 16:03:27 -0800 Subject: [PATCH 01/14] Validator service improvements --- pxtblocks/code-validation/runValidatorPlanAsync.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) 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 { From eb39f55f86f627ae0ffe85e00e774d4b125440a0 Mon Sep 17 00:00:00 2001 From: Thomas Sparks Date: Tue, 30 Jan 2024 16:33:21 -0800 Subject: [PATCH 02/14] Untested code to write / read rubrics from indexeddb --- teachertool/src/services/indexedDbService.ts | 101 +++++++++++++++++++ teachertool/src/transforms/importRubric.ts | 9 ++ teachertool/src/types/criteria.ts | 2 +- teachertool/src/types/errorCode.ts | 3 + 4 files changed, 114 insertions(+), 1 deletion(-) create mode 100644 teachertool/src/services/indexedDbService.ts create mode 100644 teachertool/src/transforms/importRubric.ts diff --git a/teachertool/src/services/indexedDbService.ts b/teachertool/src/services/indexedDbService.ts new file mode 100644 index 000000000000..d7400fb13bbb --- /dev/null +++ b/teachertool/src/services/indexedDbService.ts @@ -0,0 +1,101 @@ +import { openDB, IDBPDatabase } from "idb"; +import { ErrorCode } from "../types/errorCode"; +import { logError } from "./loggingService"; +import { CriteriaInstance } from "../types/criteria"; + +const teacherToolDbName = "makecode-project-insights"; +const dbVersion = 1; +const rubricsStoreName = "rubrics" +const lastActiveRubricKey = "_lastActiveRubricName"; + +class TeacherToolDb { + db: IDBPDatabase | undefined; + + public async initializeAsync() { + if (this.db) return; + this.db = await openDB(teacherToolDbName, dbVersion, { + upgrade(db) { + db.createObjectStore(rubricsStoreName); + }, + }); + } + + 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, key: string, value: T): Promise { + if (!this.db) { + throw new Error("IndexedDb not initialized."); + } + + try { + await this.db.put(storeName, value, key); + } 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); + } + } + + public getLastActiveRubricNameAsync(): Promise { + return this.getAsync(rubricsStoreName, lastActiveRubricKey); + } + + public getRubricWithNameAsync(name: string): Promise { + return this.getAsync(rubricsStoreName, name); + } + + public setLastActiveRubricNameAsync(name: string): Promise { + return this.setAsync(rubricsStoreName, lastActiveRubricKey, name); + } + + public setRubricWithNameAsync(name: string, rubric: CriteriaInstance[]): Promise { + return this.setAsync(rubricsStoreName, name, rubric); + } +} + +const db = new TeacherToolDb(); + +let initializeAsync = async () => { + initializeAsync = async () => {}; // Ensure future initializeAsync calls are no-ops. + + await db.initializeAsync(); +}; + +export async function getLastActiveRubricAsync(): Promise { + await initializeAsync(); + + let rubric: CriteriaInstance[] | undefined = undefined; + const name = await db.getLastActiveRubricNameAsync(); + if (name) { + rubric = await db.getRubricWithNameAsync(name); + } + + return rubric; +} + +export async function saveLastActiveRubricAsync(name: string, rubric: CriteriaInstance[]) { + await db.setRubricWithNameAsync(name, rubric); + await db.setLastActiveRubricNameAsync(name); +} \ No newline at end of file diff --git a/teachertool/src/transforms/importRubric.ts b/teachertool/src/transforms/importRubric.ts new file mode 100644 index 000000000000..538cbe321ca9 --- /dev/null +++ b/teachertool/src/transforms/importRubric.ts @@ -0,0 +1,9 @@ +import { stateAndDispatch } from "../state"; +import { CriteriaInstance } from "../types/criteria"; +import * as Actions from "../state/actions"; + +export function importRubric(serializedRubric: string) { + const { dispatch } = stateAndDispatch(); + const selectedCriteria = JSON.parse(serializedRubric) as CriteriaInstance[]; + dispatch(Actions.setSelectedCriteria(selectedCriteria)); +} \ No newline at end of file diff --git a/teachertool/src/types/criteria.ts b/teachertool/src/types/criteria.ts index 7fd5657a8f2d..9b12a3c88c21 100644 --- a/teachertool/src/types/criteria.ts +++ b/teachertool/src/types/criteria.ts @@ -34,4 +34,4 @@ export enum CriteriaEvaluationResult { Fail, CompleteWithNoResult, InProgress, -} +} \ No newline at end of file diff --git a/teachertool/src/types/errorCode.ts b/teachertool/src/types/errorCode.ts index c82dc7416c4f..27d26e8abd28 100644 --- a/teachertool/src/types/errorCode.ts +++ b/teachertool/src/types/errorCode.ts @@ -7,4 +7,7 @@ export enum ErrorCode { evalMissingCriteria = "evalMissingCriteria", evalMissingPlan = "evalMissingPlan", loadCollectionFileFailed = "loadCollectionFileFailed", + unableToGetIndexedDbRecord = "unableToGetIndexedDbRecord", + unableToSetIndexedDbRecord = "unableToSetIndexedDbRecord", + unableToDeleteIndexedDbRecord = "unableToDeleteIndexedDbRecord", } From ac06f95a374646096eee8e12963bd6a6e9e1fbcd Mon Sep 17 00:00:00 2001 From: Thomas Sparks Date: Tue, 30 Jan 2024 17:23:54 -0800 Subject: [PATCH 03/14] Create Rubric object, which also contains a field for name --- .../src/components/ActiveRubricDisplay.tsx | 23 +++++++++++++++---- .../src/components/EvalResultDisplay.tsx | 2 +- teachertool/src/services/indexedDbService.ts | 23 ++++++++++--------- teachertool/src/state/actions.ts | 12 ++++++++++ teachertool/src/state/reducer.ts | 19 +++++++++++++-- teachertool/src/state/state.ts | 5 ++-- .../src/transforms/addCriteriaToRubric.ts | 2 +- teachertool/src/transforms/importRubric.ts | 4 ++-- .../src/transforms/runEvaluateAsync.ts | 4 ++-- teachertool/src/transforms/setRubricName.ts | 7 ++++++ teachertool/src/types/rubric.ts | 6 +++++ 11 files changed, 82 insertions(+), 25 deletions(-) create mode 100644 teachertool/src/transforms/setRubricName.ts create mode 100644 teachertool/src/types/rubric.ts diff --git a/teachertool/src/components/ActiveRubricDisplay.tsx b/teachertool/src/components/ActiveRubricDisplay.tsx index 6ca897c6c70d..e1a7f6b414c3 100644 --- a/teachertool/src/components/ActiveRubricDisplay.tsx +++ b/teachertool/src/components/ActiveRubricDisplay.tsx @@ -1,21 +1,36 @@ /// -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 { Input } from "react-common/components/controls/Input"; +import { setRubricName } from "../state/actions"; interface IProps {} export const ActiveRubricDisplay: React.FC = ({}) => { - const { state: teacherTool, dispatch } = useContext(AppStateContext); + const { state: teacherTool } = useContext(AppStateContext); + + const [inProgressName, setInProgressName] = useState(teacherTool.rubric.name); + + function handleConfirmName() { + setRubricName(inProgressName); + } 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/EvalResultDisplay.tsx b/teachertool/src/components/EvalResultDisplay.tsx index 5f4afe038ade..6bfc9b4b17e1 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 index d7400fb13bbb..6ab11978db01 100644 --- a/teachertool/src/services/indexedDbService.ts +++ b/teachertool/src/services/indexedDbService.ts @@ -2,6 +2,7 @@ import { openDB, IDBPDatabase } from "idb"; import { ErrorCode } from "../types/errorCode"; import { logError } from "./loggingService"; import { CriteriaInstance } from "../types/criteria"; +import { Rubric } from "../types/rubric"; const teacherToolDbName = "makecode-project-insights"; const dbVersion = 1; @@ -62,16 +63,16 @@ class TeacherToolDb { return this.getAsync(rubricsStoreName, lastActiveRubricKey); } - public getRubricWithNameAsync(name: string): Promise { - return this.getAsync(rubricsStoreName, name); + public getRubricWithNameAsync(name: string): Promise { + return this.getAsync(rubricsStoreName, name); } - public setLastActiveRubricNameAsync(name: string): Promise { + public saveLastActiveRubricNameAsync(name: string): Promise { return this.setAsync(rubricsStoreName, lastActiveRubricKey, name); } - public setRubricWithNameAsync(name: string, rubric: CriteriaInstance[]): Promise { - return this.setAsync(rubricsStoreName, name, rubric); + public saveRubric(rubric: Rubric): Promise { + return this.setAsync(rubricsStoreName, rubric.name, rubric); } } @@ -83,10 +84,10 @@ let initializeAsync = async () => { await db.initializeAsync(); }; -export async function getLastActiveRubricAsync(): Promise { +export async function getLastActiveRubricAsync(): Promise { await initializeAsync(); - let rubric: CriteriaInstance[] | undefined = undefined; + let rubric: Rubric | undefined = undefined; const name = await db.getLastActiveRubricNameAsync(); if (name) { rubric = await db.getRubricWithNameAsync(name); @@ -95,7 +96,7 @@ export async function getLastActiveRubricAsync(): Promise ( instanceId, }); +const setRubricName = (name: string): SetRubricName => ({ + type: "SET_RUBRIC_NAME", + name, +}); + const showModal = (modal: ModalType): ShowModal => ({ type: "SHOW_MODAL", modal, @@ -170,6 +181,7 @@ export { setCatalog, setSelectedCriteria, removeCriteriaInstance, + setRubricName, showModal, hideModal, setValidatorPlans, diff --git a/teachertool/src/state/reducer.ts b/teachertool/src/state/reducer.ts index 99c91ec604ed..f80767d9b198 100644 --- a/teachertool/src/state/reducer.ts +++ b/teachertool/src/state/reducer.ts @@ -62,13 +62,28 @@ export default function reducer(state: AppState, action: Action): AppState { case "SET_SELECTED_CRITERIA": { return { ...state, - selectedCriteria: [...action.criteria], + rubric: { + ...state.rubric, + criteria: action.criteria, + }, }; } case "REMOVE_CRITERIA_INSTANCE": { return { ...state, - selectedCriteria: state.selectedCriteria.filter(c => c.instanceId !== action.instanceId), + rubric: { + ...state.rubric, + criteria: state.rubric?.criteria.filter(c => c.instanceId !== action.instanceId), + }, + }; + } + case "SET_RUBRIC_NAME": { + return { + ...state, + rubric: { + ...state.rubric, + name: action.name, + }, }; } case "SHOW_MODAL": { diff --git a/teachertool/src/state/state.ts b/teachertool/src/state/state.ts index c8fa919c8e3c..fb1c55a61632 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..21a6f0aa7740 100644 --- a/teachertool/src/transforms/addCriteriaToRubric.ts +++ b/teachertool/src/transforms/addCriteriaToRubric.ts @@ -10,7 +10,7 @@ export function addCriteriaToRubric(catalogCriteriaIds: string[]) { const { state: teacherTool, dispatch } = stateAndDispatch(); // Create instances for each of the catalog criteria. - const newSelectedCriteria = [...(teacherTool.selectedCriteria ?? [])]; + const newSelectedCriteria = [...(teacherTool.rubric.criteria ?? [])]; for (const catalogCriteriaId of catalogCriteriaIds) { const catalogCriteria = getCatalogCriteriaWithId(catalogCriteriaId); if (!catalogCriteria) { diff --git a/teachertool/src/transforms/importRubric.ts b/teachertool/src/transforms/importRubric.ts index 538cbe321ca9..2bb5cbf3d97e 100644 --- a/teachertool/src/transforms/importRubric.ts +++ b/teachertool/src/transforms/importRubric.ts @@ -4,6 +4,6 @@ import * as Actions from "../state/actions"; export function importRubric(serializedRubric: string) { const { dispatch } = stateAndDispatch(); - const selectedCriteria = JSON.parse(serializedRubric) as CriteriaInstance[]; - dispatch(Actions.setSelectedCriteria(selectedCriteria)); + + // todo : setRubric action. } \ No newline at end of file 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..4f3b2431472e --- /dev/null +++ b/teachertool/src/transforms/setRubricName.ts @@ -0,0 +1,7 @@ +import { stateAndDispatch } from "../state"; +import * as Actions from "../state/actions"; + +export function setRubricName(name: string) { + const { dispatch } = stateAndDispatch(); + dispatch(Actions.setRubricName(name)); +} \ No newline at end of file diff --git a/teachertool/src/types/rubric.ts b/teachertool/src/types/rubric.ts new file mode 100644 index 000000000000..2a9b84413654 --- /dev/null +++ b/teachertool/src/types/rubric.ts @@ -0,0 +1,6 @@ +import { CriteriaInstance } from "./criteria"; + +export interface Rubric { + name: string; + criteria: CriteriaInstance[]; +} \ No newline at end of file From f07434f9ef14404f24120e419e278f68dba50ad1 Mon Sep 17 00:00:00 2001 From: Thomas Sparks Date: Tue, 30 Jan 2024 18:06:14 -0800 Subject: [PATCH 04/14] Some saving and loading from indexeddb. May want to rework so saving happens in a centralized place that listens for rubric state changes. Also still need to save when a criteria is removed. --- teachertool/src/App.tsx | 3 ++ .../src/components/ActiveRubricDisplay.tsx | 2 +- teachertool/src/services/indexedDbService.ts | 22 ++++++++----- teachertool/src/state/actions.ts | 31 ++++++------------- teachertool/src/state/reducer.ts | 16 ++-------- .../src/transforms/addCriteriaToRubric.ts | 13 ++++++-- teachertool/src/transforms/importRubric.ts | 9 ------ teachertool/src/transforms/setRubricName.ts | 28 +++++++++++++++-- .../tryLoadLastActiveRubricAsync.ts | 17 ++++++++++ 9 files changed, 83 insertions(+), 58 deletions(-) delete mode 100644 teachertool/src/transforms/importRubric.ts create mode 100644 teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts 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 e1a7f6b414c3..1d6572ab1479 100644 --- a/teachertool/src/components/ActiveRubricDisplay.tsx +++ b/teachertool/src/components/ActiveRubricDisplay.tsx @@ -7,7 +7,7 @@ import { Button } from "react-common/components/controls/Button"; import { removeCriteriaFromRubric } from "../transforms/removeCriteriaFromRubric"; import { showCatalogModal } from "../transforms/showCatalogModal"; import { Input } from "react-common/components/controls/Input"; -import { setRubricName } from "../state/actions"; +import { setRubricName } from "../transforms/setRubricName"; interface IProps {} diff --git a/teachertool/src/services/indexedDbService.ts b/teachertool/src/services/indexedDbService.ts index 6ab11978db01..6edc69f6f825 100644 --- a/teachertool/src/services/indexedDbService.ts +++ b/teachertool/src/services/indexedDbService.ts @@ -6,7 +6,7 @@ import { Rubric } from "../types/rubric"; const teacherToolDbName = "makecode-project-insights"; const dbVersion = 1; -const rubricsStoreName = "rubrics" +const rubricsStoreName = "rubrics"; const lastActiveRubricKey = "_lastActiveRubricName"; class TeacherToolDb { @@ -63,17 +63,21 @@ class TeacherToolDb { return this.getAsync(rubricsStoreName, lastActiveRubricKey); } - public getRubricWithNameAsync(name: string): Promise { - return this.getAsync(rubricsStoreName, name); - } - public saveLastActiveRubricNameAsync(name: string): Promise { return this.setAsync(rubricsStoreName, lastActiveRubricKey, name); } + public getRubric(name: string): Promise { + return this.getAsync(rubricsStoreName, name); + } + public saveRubric(rubric: Rubric): Promise { return this.setAsync(rubricsStoreName, rubric.name, rubric); } + + public deleteRubric(name: string): Promise { + return this.deleteAsync(rubricsStoreName, name); + } } const db = new TeacherToolDb(); @@ -90,13 +94,17 @@ export async function getLastActiveRubricAsync(): Promise { let rubric: Rubric | undefined = undefined; const name = await db.getLastActiveRubricNameAsync(); if (name) { - rubric = await db.getRubricWithNameAsync(name); + rubric = await db.getRubric(name); } return rubric; } -export async function saveLastActiveRubricAsync(rubric: Rubric) { +export async function saveRubric(rubric: Rubric) { await db.saveRubric(rubric); await db.saveLastActiveRubricNameAsync(rubric.name); } + +export async function deleteRubric(name: string) { + await db.deleteRubric(name); +} diff --git a/teachertool/src/state/actions.ts b/teachertool/src/state/actions.ts index bdcaddfc264d..eaff5ec9cfe1 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,9 +50,9 @@ type SetCatalog = ActionBase & { catalog: CatalogCriteria[] | undefined; }; -type SetSelectedCriteria = ActionBase & { - type: "SET_SELECTED_CRITERIA"; - criteria: CriteriaInstance[]; +type SetRubric = ActionBase & { + type: "SET_RUBRIC"; + rubric: Rubric; }; type RemoveCriteriaInstance = ActionBase & { @@ -59,11 +60,6 @@ type RemoveCriteriaInstance = ActionBase & { instanceId: string; }; -type SetRubricName = ActionBase & { - type: "SET_RUBRIC_NAME"; - name: string; -}; - type ShowModal = ActionBase & { type: "SHOW_MODAL"; modal: ModalType; @@ -91,9 +87,8 @@ export type Action = | ClearAllEvalResults | SetTargetConfig | SetCatalog - | SetSelectedCriteria + | SetRubric | RemoveCriteriaInstance - | SetRubricName | ShowModal | HideModal | SetValidatorPlans; @@ -141,9 +136,9 @@ const setCatalog = (catalog: CatalogCriteria[] | undefined): SetCatalog => ({ catalog, }); -const setSelectedCriteria = (criteria: CriteriaInstance[]): SetSelectedCriteria => ({ - type: "SET_SELECTED_CRITERIA", - criteria, +const setRubric = (rubric: Rubric): SetRubric => ({ + type: "SET_RUBRIC", + rubric, }); const removeCriteriaInstance = (instanceId: string): RemoveCriteriaInstance => ({ @@ -151,11 +146,6 @@ const removeCriteriaInstance = (instanceId: string): RemoveCriteriaInstance => ( instanceId, }); -const setRubricName = (name: string): SetRubricName => ({ - type: "SET_RUBRIC_NAME", - name, -}); - const showModal = (modal: ModalType): ShowModal => ({ type: "SHOW_MODAL", modal, @@ -179,9 +169,8 @@ export { clearAllEvalResults, setTargetConfig, setCatalog, - setSelectedCriteria, + setRubric, removeCriteriaInstance, - setRubricName, showModal, hideModal, setValidatorPlans, diff --git a/teachertool/src/state/reducer.ts b/teachertool/src/state/reducer.ts index f80767d9b198..8a9445f4a667 100644 --- a/teachertool/src/state/reducer.ts +++ b/teachertool/src/state/reducer.ts @@ -59,13 +59,10 @@ export default function reducer(state: AppState, action: Action): AppState { catalog: action.catalog, }; } - case "SET_SELECTED_CRITERIA": { + case "SET_RUBRIC": { return { ...state, - rubric: { - ...state.rubric, - criteria: action.criteria, - }, + rubric: action.rubric, }; } case "REMOVE_CRITERIA_INSTANCE": { @@ -77,15 +74,6 @@ export default function reducer(state: AppState, action: Action): AppState { }, }; } - case "SET_RUBRIC_NAME": { - return { - ...state, - rubric: { - ...state.rubric, - name: action.name, - }, - }; - } case "SHOW_MODAL": { return { ...state, diff --git a/teachertool/src/transforms/addCriteriaToRubric.ts b/teachertool/src/transforms/addCriteriaToRubric.ts index 21a6f0aa7740..e8aa7f3d14e1 100644 --- a/teachertool/src/transforms/addCriteriaToRubric.ts +++ b/teachertool/src/transforms/addCriteriaToRubric.ts @@ -5,12 +5,17 @@ import { logDebug, logError } from "../services/loggingService"; import { CriteriaInstance, CriteriaParameterValue } from "../types/criteria"; import { nanoid } from "nanoid"; import { ErrorCode } from "../types/errorCode"; +import { saveRubric } from "../services/indexedDbService"; export function addCriteriaToRubric(catalogCriteriaIds: string[]) { const { state: teacherTool, dispatch } = stateAndDispatch(); // Create instances for each of the catalog criteria. - const newSelectedCriteria = [...(teacherTool.rubric.criteria ?? [])]; + const newRubric = { + ...teacherTool.rubric, + criteria: [...(teacherTool.rubric.criteria ?? [])], + } + for (const catalogCriteriaId of catalogCriteriaIds) { const catalogCriteria = getCatalogCriteriaWithId(catalogCriteriaId); if (!catalogCriteria) { @@ -37,12 +42,14 @@ 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), }); + + saveRubric(newRubric); // fire and forget, we don't really need to wait on this. } diff --git a/teachertool/src/transforms/importRubric.ts b/teachertool/src/transforms/importRubric.ts deleted file mode 100644 index 2bb5cbf3d97e..000000000000 --- a/teachertool/src/transforms/importRubric.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { stateAndDispatch } from "../state"; -import { CriteriaInstance } from "../types/criteria"; -import * as Actions from "../state/actions"; - -export function importRubric(serializedRubric: string) { - const { dispatch } = stateAndDispatch(); - - // todo : setRubric action. -} \ No newline at end of file diff --git a/teachertool/src/transforms/setRubricName.ts b/teachertool/src/transforms/setRubricName.ts index 4f3b2431472e..5dee9215e7ae 100644 --- a/teachertool/src/transforms/setRubricName.ts +++ b/teachertool/src/transforms/setRubricName.ts @@ -1,7 +1,29 @@ +import { deleteRubric, saveRubric } from "../services/indexedDbService"; import { stateAndDispatch } from "../state"; import * as Actions from "../state/actions"; export function setRubricName(name: string) { - const { dispatch } = stateAndDispatch(); - dispatch(Actions.setRubricName(name)); -} \ No newline at end of file + const { state: teacherTool, dispatch } = stateAndDispatch(); + + const oldName = teacherTool.rubric.name; + + if (oldName === name) { + return; + } + + const newRubric = { + ...teacherTool.rubric, + name, + } + dispatch(Actions.setRubric(newRubric)); + + // To save the renamed rubric, we can simply save a version with the new name, + // and then delete the entry with the previous name. + async function saveRenamedRubric() { + await saveRubric(newRubric); + await deleteRubric(oldName); + } + + // Fire and forget. We don't need to wait for the operation to finish. + saveRenamedRubric(); +} diff --git a/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts b/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts new file mode 100644 index 000000000000..a5e0514cb0c5 --- /dev/null +++ b/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts @@ -0,0 +1,17 @@ +import { stateAndDispatch } from "../state"; +import * as Actions from "../state/actions"; +import { getLastActiveRubricAsync } from "../services/indexedDbService"; +import { logDebug } from "../services/loggingService"; + +export async function tryLoadLastActiveRubricAsync() { + const { dispatch } = stateAndDispatch(); + + const lastActiveRubric = await getLastActiveRubricAsync(); + + if (lastActiveRubric) { + logDebug(`Loading last active rubric '${lastActiveRubric.name}'...`); + dispatch(Actions.setRubric(lastActiveRubric)); + } else { + logDebug(`No last active rubric to load.`); + } +} \ No newline at end of file From db4440d1ba44863d8fbd58dcc1e11010595f8102 Mon Sep 17 00:00:00 2001 From: Thomas Sparks Date: Wed, 31 Jan 2024 10:12:19 -0800 Subject: [PATCH 05/14] Save rubric when removing criteria --- teachertool/src/state/actions.ts | 12 ------------ teachertool/src/state/reducer.ts | 9 --------- .../src/transforms/removeCriteriaFromRubric.ts | 18 +++++++++++++----- teachertool/src/types/errorCode.ts | 2 ++ 4 files changed, 15 insertions(+), 26 deletions(-) diff --git a/teachertool/src/state/actions.ts b/teachertool/src/state/actions.ts index eaff5ec9cfe1..32d23980d497 100644 --- a/teachertool/src/state/actions.ts +++ b/teachertool/src/state/actions.ts @@ -55,11 +55,6 @@ type SetRubric = ActionBase & { rubric: Rubric; }; -type RemoveCriteriaInstance = ActionBase & { - type: "REMOVE_CRITERIA_INSTANCE"; - instanceId: string; -}; - type ShowModal = ActionBase & { type: "SHOW_MODAL"; modal: ModalType; @@ -88,7 +83,6 @@ export type Action = | SetTargetConfig | SetCatalog | SetRubric - | RemoveCriteriaInstance | ShowModal | HideModal | SetValidatorPlans; @@ -141,11 +135,6 @@ const setRubric = (rubric: Rubric): SetRubric => ({ rubric, }); -const removeCriteriaInstance = (instanceId: string): RemoveCriteriaInstance => ({ - type: "REMOVE_CRITERIA_INSTANCE", - instanceId, -}); - const showModal = (modal: ModalType): ShowModal => ({ type: "SHOW_MODAL", modal, @@ -170,7 +159,6 @@ export { setTargetConfig, setCatalog, setRubric, - removeCriteriaInstance, showModal, hideModal, setValidatorPlans, diff --git a/teachertool/src/state/reducer.ts b/teachertool/src/state/reducer.ts index 8a9445f4a667..d454881e4861 100644 --- a/teachertool/src/state/reducer.ts +++ b/teachertool/src/state/reducer.ts @@ -65,15 +65,6 @@ export default function reducer(state: AppState, action: Action): AppState { rubric: action.rubric, }; } - case "REMOVE_CRITERIA_INSTANCE": { - return { - ...state, - rubric: { - ...state.rubric, - criteria: state.rubric?.criteria.filter(c => c.instanceId !== action.instanceId), - }, - }; - } case "SHOW_MODAL": { return { ...state, diff --git a/teachertool/src/transforms/removeCriteriaFromRubric.ts b/teachertool/src/transforms/removeCriteriaFromRubric.ts index 6ab543e0d513..536155cd889b 100644 --- a/teachertool/src/transforms/removeCriteriaFromRubric.ts +++ b/teachertool/src/transforms/removeCriteriaFromRubric.ts @@ -2,13 +2,21 @@ import { stateAndDispatch } from "../state"; import * as Actions from "../state/actions"; import { logDebug } from "../services/loggingService"; import { CriteriaInstance } from "../types/criteria"; +import { saveRubric } from "../services/indexedDbService"; 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)); + + saveRubric(newRubric); // fire and forget, we don't need to wait for this to finish. + + pxt.tickEvent("teachertool.removecriteria", { catalogCriteriaId: instance.catalogCriteriaId }); } diff --git a/teachertool/src/types/errorCode.ts b/teachertool/src/types/errorCode.ts index 27d26e8abd28..a56f0dcef66b 100644 --- a/teachertool/src/types/errorCode.ts +++ b/teachertool/src/types/errorCode.ts @@ -10,4 +10,6 @@ export enum ErrorCode { unableToGetIndexedDbRecord = "unableToGetIndexedDbRecord", unableToSetIndexedDbRecord = "unableToSetIndexedDbRecord", unableToDeleteIndexedDbRecord = "unableToDeleteIndexedDbRecord", + loadUnrecognizedCatalogId = "loadUnrecognizedCatalogId", + loadUnrecognizedParameter = "loadUnrecognizedParameter", } From 3d531a48de284cbef71907ce5c07396a282111e6 Mon Sep 17 00:00:00 2001 From: Thomas Sparks Date: Wed, 31 Jan 2024 10:12:28 -0800 Subject: [PATCH 06/14] Validate criteria when loading --- .../tryLoadLastActiveRubricAsync.ts | 43 ++++++++++++++++++- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts b/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts index a5e0514cb0c5..d96f8d07c1f1 100644 --- a/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts +++ b/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts @@ -1,7 +1,38 @@ import { stateAndDispatch } from "../state"; import * as Actions from "../state/actions"; import { getLastActiveRubricAsync } from "../services/indexedDbService"; -import { logDebug } from "../services/loggingService"; +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(); @@ -10,8 +41,16 @@ export async function tryLoadLastActiveRubricAsync() { 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("Some criteria could not be loaded.", 2000)); + } + dispatch(Actions.setRubric(lastActiveRubric)); } else { logDebug(`No last active rubric to load.`); } -} \ No newline at end of file +} From 5068f1c835d93fe3643b4fcd88fa47039e811088 Mon Sep 17 00:00:00 2001 From: Thomas Sparks Date: Wed, 31 Jan 2024 10:46:44 -0800 Subject: [PATCH 07/14] Move save code into transform called by reducer --- teachertool/src/state/reducer.ts | 2 ++ teachertool/src/transforms/addCriteriaToRubric.ts | 3 --- .../src/transforms/removeCriteriaFromRubric.ts | 3 --- teachertool/src/transforms/setRubricName.ts | 11 ----------- teachertool/src/transforms/updateStoredRubric.ts | 15 +++++++++++++++ 5 files changed, 17 insertions(+), 17 deletions(-) create mode 100644 teachertool/src/transforms/updateStoredRubric.ts diff --git a/teachertool/src/state/reducer.ts b/teachertool/src/state/reducer.ts index d454881e4861..14e2234725ee 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. @@ -60,6 +61,7 @@ export default function reducer(state: AppState, action: Action): AppState { }; } case "SET_RUBRIC": { + updateStoredRubricAsync(state.rubric, action.rubric); // fire and forget, we don't need to wait for this to finish. return { ...state, rubric: action.rubric, diff --git a/teachertool/src/transforms/addCriteriaToRubric.ts b/teachertool/src/transforms/addCriteriaToRubric.ts index e8aa7f3d14e1..b936b2a73c44 100644 --- a/teachertool/src/transforms/addCriteriaToRubric.ts +++ b/teachertool/src/transforms/addCriteriaToRubric.ts @@ -5,7 +5,6 @@ import { logDebug, logError } from "../services/loggingService"; import { CriteriaInstance, CriteriaParameterValue } from "../types/criteria"; import { nanoid } from "nanoid"; import { ErrorCode } from "../types/errorCode"; -import { saveRubric } from "../services/indexedDbService"; export function addCriteriaToRubric(catalogCriteriaIds: string[]) { const { state: teacherTool, dispatch } = stateAndDispatch(); @@ -50,6 +49,4 @@ export function addCriteriaToRubric(catalogCriteriaIds: string[]) { pxt.tickEvent("teachertool.addcriteria", { ids: JSON.stringify(catalogCriteriaIds), }); - - saveRubric(newRubric); // fire and forget, we don't really need to wait on this. } diff --git a/teachertool/src/transforms/removeCriteriaFromRubric.ts b/teachertool/src/transforms/removeCriteriaFromRubric.ts index 536155cd889b..1dfb4f01ed60 100644 --- a/teachertool/src/transforms/removeCriteriaFromRubric.ts +++ b/teachertool/src/transforms/removeCriteriaFromRubric.ts @@ -2,7 +2,6 @@ import { stateAndDispatch } from "../state"; import * as Actions from "../state/actions"; import { logDebug } from "../services/loggingService"; import { CriteriaInstance } from "../types/criteria"; -import { saveRubric } from "../services/indexedDbService"; export function removeCriteriaFromRubric(instance: CriteriaInstance) { const { state: teacherTool, dispatch } = stateAndDispatch(); @@ -16,7 +15,5 @@ export function removeCriteriaFromRubric(instance: CriteriaInstance) { dispatch(Actions.setRubric(newRubric)); - saveRubric(newRubric); // fire and forget, we don't need to wait for this to finish. - pxt.tickEvent("teachertool.removecriteria", { catalogCriteriaId: instance.catalogCriteriaId }); } diff --git a/teachertool/src/transforms/setRubricName.ts b/teachertool/src/transforms/setRubricName.ts index 5dee9215e7ae..002cee02e077 100644 --- a/teachertool/src/transforms/setRubricName.ts +++ b/teachertool/src/transforms/setRubricName.ts @@ -1,4 +1,3 @@ -import { deleteRubric, saveRubric } from "../services/indexedDbService"; import { stateAndDispatch } from "../state"; import * as Actions from "../state/actions"; @@ -16,14 +15,4 @@ export function setRubricName(name: string) { name, } dispatch(Actions.setRubric(newRubric)); - - // To save the renamed rubric, we can simply save a version with the new name, - // and then delete the entry with the previous name. - async function saveRenamedRubric() { - await saveRubric(newRubric); - await deleteRubric(oldName); - } - - // Fire and forget. We don't need to wait for the operation to finish. - saveRenamedRubric(); } diff --git a/teachertool/src/transforms/updateStoredRubric.ts b/teachertool/src/transforms/updateStoredRubric.ts new file mode 100644 index 000000000000..a0c94096463d --- /dev/null +++ b/teachertool/src/transforms/updateStoredRubric.ts @@ -0,0 +1,15 @@ +import { deleteRubric, saveRubric } 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 saveRubric(newRubric); + } + + if (renamed || deleted) { + await deleteRubric(oldRubric.name); + } +} From 1dfc266d027b448d02be44ba2e2fcc2c4c73f0ef Mon Sep 17 00:00:00 2001 From: Thomas Sparks Date: Wed, 31 Jan 2024 12:53:44 -0800 Subject: [PATCH 08/14] Update schema for indexeddb to separate metadata. I anticipate we may have additional fields we'd want to store here, like autosave and autorun preferences. --- teachertool/src/services/indexedDbService.ts | 40 ++++++++++++++----- .../src/transforms/updateStoredRubric.ts | 6 +-- 2 files changed, 32 insertions(+), 14 deletions(-) diff --git a/teachertool/src/services/indexedDbService.ts b/teachertool/src/services/indexedDbService.ts index 6edc69f6f825..bc7571d7e17e 100644 --- a/teachertool/src/services/indexedDbService.ts +++ b/teachertool/src/services/indexedDbService.ts @@ -1,13 +1,17 @@ import { openDB, IDBPDatabase } from "idb"; import { ErrorCode } from "../types/errorCode"; import { logError } from "./loggingService"; -import { CriteriaInstance } from "../types/criteria"; import { Rubric } from "../types/rubric"; const teacherToolDbName = "makecode-project-insights"; const dbVersion = 1; const rubricsStoreName = "rubrics"; -const lastActiveRubricKey = "_lastActiveRubricName"; +const metadataStoreName = "metadata"; +const metadataKeys = { + lastActiveRubricKey: "lastActiveRubricName", +}; + +type MetadataEntry = { key: string; value: any }; class TeacherToolDb { db: IDBPDatabase | undefined; @@ -16,7 +20,8 @@ class TeacherToolDb { if (this.db) return; this.db = await openDB(teacherToolDbName, dbVersion, { upgrade(db) { - db.createObjectStore(rubricsStoreName); + db.createObjectStore(rubricsStoreName, { keyPath: "name" }); + db.createObjectStore(metadataStoreName, { keyPath: "key" }); }, }); } @@ -34,13 +39,13 @@ class TeacherToolDb { } } - private async setAsync(storeName: string, key: string, value: T): Promise { + private async setAsync(storeName: string, value: T): Promise { if (!this.db) { throw new Error("IndexedDb not initialized."); } try { - await this.db.put(storeName, value, key); + await this.db.put(storeName, value); } catch (e) { // Not recording key, as it could contain user-input with sensitive information. logError(ErrorCode.unableToSetIndexedDbRecord, e); @@ -59,12 +64,25 @@ class TeacherToolDb { } } - public getLastActiveRubricNameAsync(): Promise { - return this.getAsync(rubricsStoreName, lastActiveRubricKey); + 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.setAsync(rubricsStoreName, lastActiveRubricKey, name); + return this.setMetadataEntryAsync(metadataKeys.lastActiveRubricKey, name); } public getRubric(name: string): Promise { @@ -72,7 +90,7 @@ class TeacherToolDb { } public saveRubric(rubric: Rubric): Promise { - return this.setAsync(rubricsStoreName, rubric.name, rubric); + return this.setAsync(rubricsStoreName, rubric); } public deleteRubric(name: string): Promise { @@ -100,11 +118,11 @@ export async function getLastActiveRubricAsync(): Promise { return rubric; } -export async function saveRubric(rubric: Rubric) { +export async function saveRubricAsync(rubric: Rubric) { await db.saveRubric(rubric); await db.saveLastActiveRubricNameAsync(rubric.name); } -export async function deleteRubric(name: string) { +export async function deleteRubricAsync(name: string) { await db.deleteRubric(name); } diff --git a/teachertool/src/transforms/updateStoredRubric.ts b/teachertool/src/transforms/updateStoredRubric.ts index a0c94096463d..1533a9928820 100644 --- a/teachertool/src/transforms/updateStoredRubric.ts +++ b/teachertool/src/transforms/updateStoredRubric.ts @@ -1,4 +1,4 @@ -import { deleteRubric, saveRubric } from "../services/indexedDbService"; +import { deleteRubricAsync, saveRubricAsync } from "../services/indexedDbService"; import { Rubric } from "../types/rubric"; export async function updateStoredRubricAsync(oldRubric: Rubric | undefined, newRubric: Rubric | undefined) { @@ -6,10 +6,10 @@ export async function updateStoredRubricAsync(oldRubric: Rubric | undefined, new const deleted = oldRubric && !newRubric; if (newRubric) { - await saveRubric(newRubric); + await saveRubricAsync(newRubric); } if (renamed || deleted) { - await deleteRubric(oldRubric.name); + await deleteRubricAsync(oldRubric.name); } } From 8d0e79cc63c9304c0a714f4403e5def0488eb3c6 Mon Sep 17 00:00:00 2001 From: Thomas Sparks Date: Wed, 31 Jan 2024 13:01:56 -0800 Subject: [PATCH 09/14] Prettier --- teachertool/src/components/DebugInput.tsx | 2 +- .../src/components/EvalResultDisplay.tsx | 32 +++++++++++-------- teachertool/src/state/state.ts | 2 +- .../src/transforms/addCriteriaToRubric.ts | 2 +- teachertool/src/transforms/setRubricName.ts | 2 +- teachertool/src/types/criteria.ts | 2 +- teachertool/src/types/errorCode.ts | 2 +- teachertool/src/types/rubric.ts | 2 +- 8 files changed, 25 insertions(+), 21 deletions(-) diff --git a/teachertool/src/components/DebugInput.tsx b/teachertool/src/components/DebugInput.tsx index 22df0914a07e..a674d008badf 100644 --- a/teachertool/src/components/DebugInput.tsx +++ b/teachertool/src/components/DebugInput.tsx @@ -38,4 +38,4 @@ export const DebugInput: React.FC = ({}) => { />
); -}; \ No newline at end of file +}; diff --git a/teachertool/src/components/EvalResultDisplay.tsx b/teachertool/src/components/EvalResultDisplay.tsx index 6bfc9b4b17e1..d66fdab0999c 100644 --- a/teachertool/src/components/EvalResultDisplay.tsx +++ b/teachertool/src/components/EvalResultDisplay.tsx @@ -28,20 +28,24 @@ export const EvalResultDisplay: React.FC = ({}) => { {Object.keys(teacherTool.evalResults ?? {}).map(criteriaInstanceId => { const result = teacherTool.evalResults[criteriaInstanceId]; const label = getTemplateStringFromCriteriaInstanceId(criteriaInstanceId); - return label && ( -
-

- {getTemplateStringFromCriteriaInstanceId(criteriaInstanceId)}: -

- {result === CriteriaEvaluationResult.InProgress &&
} - {result === CriteriaEvaluationResult.CompleteWithNoResult &&

{lf("N/A")}

} - {result === CriteriaEvaluationResult.Fail && ( -

{lf("Needs Work")}

- )} - {result === CriteriaEvaluationResult.Pass && ( -

{lf("Looks Good!")}

- )} -
+ return ( + label && ( +
+

+ {getTemplateStringFromCriteriaInstanceId(criteriaInstanceId)}: +

+ {result === CriteriaEvaluationResult.InProgress && ( +
+ )} + {result === CriteriaEvaluationResult.CompleteWithNoResult &&

{lf("N/A")}

} + {result === CriteriaEvaluationResult.Fail && ( +

{lf("Needs Work")}

+ )} + {result === CriteriaEvaluationResult.Pass && ( +

{lf("Looks Good!")}

+ )} +
+ ) ); })}
diff --git a/teachertool/src/state/state.ts b/teachertool/src/state/state.ts index fb1c55a61632..7180ad9c1c52 100644 --- a/teachertool/src/state/state.ts +++ b/teachertool/src/state/state.ts @@ -21,7 +21,7 @@ export const initialAppState: AppState = { evalResults: {}, projectMetadata: undefined, catalog: undefined, - rubric: {name: "", criteria: []}, + rubric: { name: "", criteria: [] }, modal: undefined, validatorPlans: undefined, flags: { diff --git a/teachertool/src/transforms/addCriteriaToRubric.ts b/teachertool/src/transforms/addCriteriaToRubric.ts index b936b2a73c44..c262831564dd 100644 --- a/teachertool/src/transforms/addCriteriaToRubric.ts +++ b/teachertool/src/transforms/addCriteriaToRubric.ts @@ -13,7 +13,7 @@ export function addCriteriaToRubric(catalogCriteriaIds: string[]) { const newRubric = { ...teacherTool.rubric, criteria: [...(teacherTool.rubric.criteria ?? [])], - } + }; for (const catalogCriteriaId of catalogCriteriaIds) { const catalogCriteria = getCatalogCriteriaWithId(catalogCriteriaId); diff --git a/teachertool/src/transforms/setRubricName.ts b/teachertool/src/transforms/setRubricName.ts index 002cee02e077..6d43a74f4fe3 100644 --- a/teachertool/src/transforms/setRubricName.ts +++ b/teachertool/src/transforms/setRubricName.ts @@ -13,6 +13,6 @@ export function setRubricName(name: string) { const newRubric = { ...teacherTool.rubric, name, - } + }; dispatch(Actions.setRubric(newRubric)); } diff --git a/teachertool/src/types/criteria.ts b/teachertool/src/types/criteria.ts index 9b12a3c88c21..7fd5657a8f2d 100644 --- a/teachertool/src/types/criteria.ts +++ b/teachertool/src/types/criteria.ts @@ -34,4 +34,4 @@ export enum CriteriaEvaluationResult { Fail, CompleteWithNoResult, InProgress, -} \ No newline at end of file +} diff --git a/teachertool/src/types/errorCode.ts b/teachertool/src/types/errorCode.ts index a56f0dcef66b..3a2a17992b12 100644 --- a/teachertool/src/types/errorCode.ts +++ b/teachertool/src/types/errorCode.ts @@ -10,6 +10,6 @@ export enum ErrorCode { unableToGetIndexedDbRecord = "unableToGetIndexedDbRecord", unableToSetIndexedDbRecord = "unableToSetIndexedDbRecord", unableToDeleteIndexedDbRecord = "unableToDeleteIndexedDbRecord", - loadUnrecognizedCatalogId = "loadUnrecognizedCatalogId", + loadUnrecognizedCatalogId = "loadUnrecognizedCatalogId", loadUnrecognizedParameter = "loadUnrecognizedParameter", } diff --git a/teachertool/src/types/rubric.ts b/teachertool/src/types/rubric.ts index 2a9b84413654..073ca325c7d0 100644 --- a/teachertool/src/types/rubric.ts +++ b/teachertool/src/types/rubric.ts @@ -3,4 +3,4 @@ import { CriteriaInstance } from "./criteria"; export interface Rubric { name: string; criteria: CriteriaInstance[]; -} \ No newline at end of file +} From 05267c0f7c4cc7b121b0a382218d63f87e4ca031 Mon Sep 17 00:00:00 2001 From: Thomas Sparks Date: Wed, 31 Jan 2024 15:06:12 -0800 Subject: [PATCH 10/14] Aria label and preserveValueOnBlur for rubric name input --- teachertool/src/components/ActiveRubricDisplay.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/teachertool/src/components/ActiveRubricDisplay.tsx b/teachertool/src/components/ActiveRubricDisplay.tsx index 1d6572ab1479..f0572cd6e070 100644 --- a/teachertool/src/components/ActiveRubricDisplay.tsx +++ b/teachertool/src/components/ActiveRubricDisplay.tsx @@ -24,11 +24,13 @@ export const ActiveRubricDisplay: React.FC = ({}) => {
{teacherTool.rubric.criteria?.map(criteriaInstance => { if (!criteriaInstance) return null; From 0619155459b185fcb144d0a67dd5b88763f76d3c Mon Sep 17 00:00:00 2001 From: Thomas Sparks Date: Wed, 31 Jan 2024 15:13:08 -0800 Subject: [PATCH 11/14] Use a global promise that initializes the db to ensure initialization happens exactly once and all code that references the db must go through the promise to get it. --- teachertool/src/services/indexedDbService.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/teachertool/src/services/indexedDbService.ts b/teachertool/src/services/indexedDbService.ts index bc7571d7e17e..3a933ae0d634 100644 --- a/teachertool/src/services/indexedDbService.ts +++ b/teachertool/src/services/indexedDbService.ts @@ -98,16 +98,14 @@ class TeacherToolDb { } } -const db = new TeacherToolDb(); - -let initializeAsync = async () => { - initializeAsync = async () => {}; // Ensure future initializeAsync calls are no-ops. - +const getDb = (async () => { + const db = new TeacherToolDb(); await db.initializeAsync(); -}; + return db; +})(); export async function getLastActiveRubricAsync(): Promise { - await initializeAsync(); + const db = await getDb; let rubric: Rubric | undefined = undefined; const name = await db.getLastActiveRubricNameAsync(); @@ -119,10 +117,12 @@ export async function getLastActiveRubricAsync(): Promise { } 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); } From d3a95cb4e9f5256ff5db4fd035d0480b891e4aaf Mon Sep 17 00:00:00 2001 From: Thomas Sparks <69657545+thsparks@users.noreply.github.com> Date: Wed, 31 Jan 2024 15:18:39 -0800 Subject: [PATCH 12/14] Apply suggestions from code review Co-authored-by: Eric Anderson --- teachertool/src/state/reducer.ts | 2 +- teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/teachertool/src/state/reducer.ts b/teachertool/src/state/reducer.ts index 14e2234725ee..7dff79b9e5eb 100644 --- a/teachertool/src/state/reducer.ts +++ b/teachertool/src/state/reducer.ts @@ -61,7 +61,7 @@ export default function reducer(state: AppState, action: Action): AppState { }; } case "SET_RUBRIC": { - updateStoredRubricAsync(state.rubric, action.rubric); // fire and forget, we don't need to wait for this to finish. + /*await*/ updateStoredRubricAsync(state.rubric, action.rubric); // fire and forget, we don't need to wait for this to finish. return { ...state, rubric: action.rubric, diff --git a/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts b/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts index d96f8d07c1f1..e8229cebd8d6 100644 --- a/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts +++ b/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts @@ -46,7 +46,7 @@ export async function tryLoadLastActiveRubricAsync() { lastActiveRubric.criteria = lastActiveRubric.criteria.filter(validateCriteriaInstance); if (lastActiveRubric.criteria.length !== initialCriteriaCount) { - postNotification(makeNotification("Some criteria could not be loaded.", 2000)); + postNotification(makeNotification(lf("Some criteria could not be loaded."), 2000)); } dispatch(Actions.setRubric(lastActiveRubric)); From cc030244a9cc41c3e9c15ab1c75d1f6b85973bef Mon Sep 17 00:00:00 2001 From: Thomas Sparks Date: Wed, 31 Jan 2024 16:46:02 -0800 Subject: [PATCH 13/14] Create & use a debounced input to save the rubric name rather than requiring the user to click away or click enter --- .../src/components/ActiveRubricDisplay.tsx | 16 +++-------- teachertool/src/components/DebouncedInput.tsx | 28 +++++++++++++++++++ 2 files changed, 32 insertions(+), 12 deletions(-) create mode 100644 teachertool/src/components/DebouncedInput.tsx diff --git a/teachertool/src/components/ActiveRubricDisplay.tsx b/teachertool/src/components/ActiveRubricDisplay.tsx index f0572cd6e070..fbf3787c49c5 100644 --- a/teachertool/src/components/ActiveRubricDisplay.tsx +++ b/teachertool/src/components/ActiveRubricDisplay.tsx @@ -6,30 +6,22 @@ import { getCatalogCriteriaWithId } from "../state/helpers"; import { Button } from "react-common/components/controls/Button"; import { removeCriteriaFromRubric } from "../transforms/removeCriteriaFromRubric"; import { showCatalogModal } from "../transforms/showCatalogModal"; -import { Input } from "react-common/components/controls/Input"; import { setRubricName } from "../transforms/setRubricName"; +import { DebouncedInput } from "./DebouncedInput"; interface IProps {} export const ActiveRubricDisplay: React.FC = ({}) => { const { state: teacherTool } = useContext(AppStateContext); - const [inProgressName, setInProgressName] = useState(teacherTool.rubric.name); - - function handleConfirmName() { - setRubricName(inProgressName); - } - return (
- {teacherTool.rubric.criteria?.map(criteriaInstance => { diff --git a/teachertool/src/components/DebouncedInput.tsx b/teachertool/src/components/DebouncedInput.tsx new file mode 100644 index 000000000000..25d39e6b0d64 --- /dev/null +++ b/teachertool/src/components/DebouncedInput.tsx @@ -0,0 +1,28 @@ +import { useRef } from "react"; +import { Input, InputProps } from "react-common/components/controls/Input"; + +export interface DebouncedInputProps extends InputProps { + intervalMs?: number | undefined; +} + +// This functions like the React Common Input, but debounces onChange calls +// so they only fire once every `interval` milliseconds (defined in props) +export const DebouncedInput: React.FC = props => { + const timerId = useRef(undefined); + + const onChangeDebounce = (newValue: string) => { + if (timerId.current) { + clearTimeout(timerId.current); + } + + timerId.current = setTimeout(() => { + if (!props.onChange) { + return; + } + + props.onChange(newValue); + }, props.intervalMs ?? 500); + }; + + return ; +}; From 3f6314cdcc09da4edb015fe478f391ef380877ac Mon Sep 17 00:00:00 2001 From: Thomas Sparks Date: Thu, 1 Feb 2024 10:04:22 -0800 Subject: [PATCH 14/14] DebouncedInput updates from PR (send event if pending and unmounting the component, move default interval value for better visibility) --- teachertool/src/components/DebouncedInput.tsx | 39 +++++++++++++------ 1 file changed, 27 insertions(+), 12 deletions(-) diff --git a/teachertool/src/components/DebouncedInput.tsx b/teachertool/src/components/DebouncedInput.tsx index 25d39e6b0d64..d9bfd5a1d79e 100644 --- a/teachertool/src/components/DebouncedInput.tsx +++ b/teachertool/src/components/DebouncedInput.tsx @@ -1,27 +1,42 @@ -import { useRef } from "react"; +import { useEffect, useRef } from "react"; import { Input, InputProps } from "react-common/components/controls/Input"; export interface DebouncedInputProps extends InputProps { - intervalMs?: number | undefined; + intervalMs?: number; // Default 500 ms } -// This functions like the React Common Input, but debounces onChange calls -// so they only fire once every `interval` milliseconds (defined in props) -export const DebouncedInput: React.FC = props => { +// 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(() => { - if (!props.onChange) { - return; - } - - props.onChange(newValue); - }, props.intervalMs ?? 500); + timerId.current = setTimeout(sendChange, intervalMs); }; return ;