From 28342dc100a82b82c32624f0291a12f9e5cead85 Mon Sep 17 00:00:00 2001 From: Thomas Sparks <69657545+thsparks@users.noreply.github.com> Date: Thu, 8 Feb 2024 16:43:37 -0800 Subject: [PATCH 1/3] Move shared teacher tool docs from docs to common-docs (#9860) We weren't able to see any shared catalog entries in beta, and apparently this is because the /docs/ folder is for actual pxt docs (makecode.com/docs) and they're not considered shared. For shared docs, we need to move them into /common-docs/. --- {docs => common-docs}/teachertool/catalog-shared.json | 0 {docs => common-docs}/teachertool/test/catalog-shared.json | 0 .../teachertool/test/validator-plans-shared.json | 0 {docs => common-docs}/teachertool/validator-plans-shared.json | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename {docs => common-docs}/teachertool/catalog-shared.json (100%) rename {docs => common-docs}/teachertool/test/catalog-shared.json (100%) rename {docs => common-docs}/teachertool/test/validator-plans-shared.json (100%) rename {docs => common-docs}/teachertool/validator-plans-shared.json (100%) diff --git a/docs/teachertool/catalog-shared.json b/common-docs/teachertool/catalog-shared.json similarity index 100% rename from docs/teachertool/catalog-shared.json rename to common-docs/teachertool/catalog-shared.json diff --git a/docs/teachertool/test/catalog-shared.json b/common-docs/teachertool/test/catalog-shared.json similarity index 100% rename from docs/teachertool/test/catalog-shared.json rename to common-docs/teachertool/test/catalog-shared.json diff --git a/docs/teachertool/test/validator-plans-shared.json b/common-docs/teachertool/test/validator-plans-shared.json similarity index 100% rename from docs/teachertool/test/validator-plans-shared.json rename to common-docs/teachertool/test/validator-plans-shared.json diff --git a/docs/teachertool/validator-plans-shared.json b/common-docs/teachertool/validator-plans-shared.json similarity index 100% rename from docs/teachertool/validator-plans-shared.json rename to common-docs/teachertool/validator-plans-shared.json From 3f419e36f4db83df2693b8ad525f75e5143b82f1 Mon Sep 17 00:00:00 2001 From: Thomas Sparks <69657545+thsparks@users.noreply.github.com> Date: Fri, 9 Feb 2024 10:23:54 -0800 Subject: [PATCH 2/3] Move Last Active Rubric Name to Browser Local Storage (#9858) I originally created a metadata table inside the Indexed DB to store this kind of info, but I didn't know about localstorage back then. Since we now have a service to handle that, I wanted to move the last active rubric name into the same area and get rid of that metadata table. --- teachertool/src/services/localStorage.ts | 36 ------ ...{indexedDbService.ts => storageService.ts} | 120 +++++++++++++----- teachertool/src/state/appStateContext.tsx | 4 +- teachertool/src/transforms/setAutorun.ts | 4 +- .../tryLoadLastActiveRubricAsync.ts | 4 +- .../src/transforms/updateStoredRubric.ts | 2 +- 6 files changed, 92 insertions(+), 78 deletions(-) delete mode 100644 teachertool/src/services/localStorage.ts rename teachertool/src/services/{indexedDbService.ts => storageService.ts} (57%) diff --git a/teachertool/src/services/localStorage.ts b/teachertool/src/services/localStorage.ts deleted file mode 100644 index ca49d45cb6de..000000000000 --- a/teachertool/src/services/localStorage.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { ErrorCode } from "../types/errorCode"; -import { logError } from "./loggingService"; - -const KEY_PREFIX = "teachertool"; -const AUTORUN_KEY = [KEY_PREFIX, "autorun"].join("/"); - -function getValue(key: string, defaultValue?: string): string | undefined { - return localStorage.getItem(key) || defaultValue; -} - -function setValue(key: string, val: string) { - localStorage.setItem(key, val); -} - -function delValue(key: string) { - localStorage.removeItem(key); -} - -function getAutorun(): boolean { - try { - return getValue(AUTORUN_KEY, "false") === "true"; - } catch (e) { - logError(ErrorCode.localStorageReadError, e); - return false; - } -} - -function setAutorun(autorun: boolean) { - try { - setValue(AUTORUN_KEY, autorun.toString()); - } catch (e) { - logError(ErrorCode.localStorageWriteError, e); - } -} - -export { getAutorun, setAutorun }; diff --git a/teachertool/src/services/indexedDbService.ts b/teachertool/src/services/storageService.ts similarity index 57% rename from teachertool/src/services/indexedDbService.ts rename to teachertool/src/services/storageService.ts index 3a933ae0d634..e0677609e6c3 100644 --- a/teachertool/src/services/indexedDbService.ts +++ b/teachertool/src/services/storageService.ts @@ -3,15 +3,33 @@ import { ErrorCode } from "../types/errorCode"; import { logError } from "./loggingService"; import { Rubric } from "../types/rubric"; +// ---------------------------------- +// Local Storage (for simple key -> value mappings of small data) +// ---------------------------------- + +const KEY_PREFIX = "teachertool"; +const AUTORUN_KEY = [KEY_PREFIX, "autorun"].join("/"); +const LAST_ACTIVE_RUBRIC_KEY = [KEY_PREFIX, "lastActiveRubric"].join("/"); + +function getValue(key: string, defaultValue?: string): string | undefined { + return localStorage.getItem(key) || defaultValue; +} + +function setValue(key: string, val: string) { + localStorage.setItem(key, val); +} + +function delValue(key: string) { + localStorage.removeItem(key); +} + +// ---------------------------------- +// Indexed DB (for storing larger, structured data) +// ---------------------------------- + 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; @@ -21,7 +39,6 @@ class TeacherToolDb { this.db = await openDB(teacherToolDbName, dbVersion, { upgrade(db) { db.createObjectStore(rubricsStoreName, { keyPath: "name" }); - db.createObjectStore(metadataStoreName, { keyPath: "key" }); }, }); } @@ -64,27 +81,6 @@ class TeacherToolDb { } } - 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); } @@ -104,11 +100,58 @@ const getDb = (async () => { return db; })(); -export async function getLastActiveRubricAsync(): Promise { +async function saveRubricToIndexedDbAsync(rubric: Rubric) { + const db = await getDb; + await db.saveRubric(rubric); +} + +async function deleteRubricFromIndexedDbAsync(name: string) { + const db = await getDb; + await db.deleteRubric(name); +} + +// ---------------------------------- +// Exports +// ---------------------------------- + +export function getAutorun(): boolean { + try { + return getValue(AUTORUN_KEY, "false") === "true"; + } catch (e) { + logError(ErrorCode.localStorageReadError, e); + return false; + } +} + +export function setAutorun(autorun: boolean) { + try { + setValue(AUTORUN_KEY, autorun.toString()); + } catch (e) { + logError(ErrorCode.localStorageWriteError, e); + } +} + +export function getLastActiveRubricName(): string { + try { + return getValue(LAST_ACTIVE_RUBRIC_KEY) ?? ""; + } catch (e) { + logError(ErrorCode.localStorageReadError, e); + return ""; + } +} + +export function setLastActiveRubricName(name: string) { + try { + setValue(LAST_ACTIVE_RUBRIC_KEY, name); + } catch (e) { + logError(ErrorCode.localStorageWriteError, e); + } +} + +export async function getRubric(name: string): Promise { const db = await getDb; let rubric: Rubric | undefined = undefined; - const name = await db.getLastActiveRubricNameAsync(); if (name) { rubric = await db.getRubric(name); } @@ -116,13 +159,20 @@ export async function getLastActiveRubricAsync(): Promise { return rubric; } +export async function getLastActiveRubricAsync(): Promise { + const lastActiveRubricName = getLastActiveRubricName(); + return await getRubric(lastActiveRubricName); +} + export async function saveRubricAsync(rubric: Rubric) { - const db = await getDb; - await db.saveRubric(rubric); - await db.saveLastActiveRubricNameAsync(rubric.name); + await saveRubricToIndexedDbAsync(rubric); + setLastActiveRubricName(rubric.name); } export async function deleteRubricAsync(name: string) { - const db = await getDb; - await db.deleteRubric(name); + await deleteRubricFromIndexedDbAsync(name); + + if (getLastActiveRubricName() === name) { + setLastActiveRubricName(""); + } } diff --git a/teachertool/src/state/appStateContext.tsx b/teachertool/src/state/appStateContext.tsx index feadcf7cc4b1..1a1eb23c7dbe 100644 --- a/teachertool/src/state/appStateContext.tsx +++ b/teachertool/src/state/appStateContext.tsx @@ -3,7 +3,7 @@ import { AppState, initialAppState } from "./state"; import { Action } from "./actions"; import reducer from "./reducer"; import assert from "assert"; -import * as LocalStorage from "../services/localStorage"; +import { getAutorun } from "../services/storageService"; let state: AppState; let dispatch: React.Dispatch; @@ -41,7 +41,7 @@ export function AppStateProvider(props: React.PropsWithChildren<{}>): React.Reac // Create the application state and state change mechanism (dispatch) const [state_, dispatch_] = useReducer(reducer, { ...initialAppState, - autorun: LocalStorage.getAutorun(), + autorun: getAutorun(), flags: { ...initialAppState.flags, testCatalog, diff --git a/teachertool/src/transforms/setAutorun.ts b/teachertool/src/transforms/setAutorun.ts index 2d40a26bbfae..df9b2f563618 100644 --- a/teachertool/src/transforms/setAutorun.ts +++ b/teachertool/src/transforms/setAutorun.ts @@ -1,11 +1,11 @@ import { stateAndDispatch } from "../state"; import * as Actions from "../state/actions"; -import * as LocalStorage from "../services/localStorage"; +import * as Storage from "../services/storageService"; import * as AutorunService from "../services/autorunService"; export function setAutorun(autorun: boolean) { const { dispatch } = stateAndDispatch(); dispatch(Actions.setAutorun(autorun)); - LocalStorage.setAutorun(autorun); + Storage.setAutorun(autorun); AutorunService.poke(); } diff --git a/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts b/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts index daf484e8b693..3bf907878113 100644 --- a/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts +++ b/teachertool/src/transforms/tryLoadLastActiveRubricAsync.ts @@ -1,5 +1,5 @@ -import { getLastActiveRubricAsync } from "../services/indexedDbService"; -import { logDebug, logError } from "../services/loggingService"; +import { getLastActiveRubricAsync } from "../services/storageService"; +import { logDebug } from "../services/loggingService"; import { showToast } from "./showToast"; import { makeToast } from "../utils"; import { setRubric } from "./setRubric"; diff --git a/teachertool/src/transforms/updateStoredRubric.ts b/teachertool/src/transforms/updateStoredRubric.ts index 1533a9928820..c2bdf1f92a53 100644 --- a/teachertool/src/transforms/updateStoredRubric.ts +++ b/teachertool/src/transforms/updateStoredRubric.ts @@ -1,4 +1,4 @@ -import { deleteRubricAsync, saveRubricAsync } from "../services/indexedDbService"; +import { deleteRubricAsync, saveRubricAsync } from "../services/storageService"; import { Rubric } from "../types/rubric"; export async function updateStoredRubricAsync(oldRubric: Rubric | undefined, newRubric: Rubric | undefined) { From 38ed80ed236e9501af9d12468e9d1a3fe25c603c Mon Sep 17 00:00:00 2001 From: Thomas Sparks <69657545+thsparks@users.noreply.github.com> Date: Fri, 9 Feb 2024 11:20:07 -0800 Subject: [PATCH 3/3] Consolidate calls to set rubric so they go through the transform. (#9861) This change sends all setRubric calls through the setRubric transform, so they don't call into the action directly. This is just a refactor; there is no change in functionality. I didn't do this originally because the setRubric transform was doing a bunch of validation that would have been redundant, but I refactored that out and forgot to go in and change these calls. Now with this change, we can rely on the transform to poke autorun and we don't need to call that separately every time. --- teachertool/src/transforms/addCriteriaToRubric.ts | 6 ++---- teachertool/src/transforms/removeCriteriaFromRubric.ts | 6 ++---- teachertool/src/transforms/setRubricName.ts | 6 +++--- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/teachertool/src/transforms/addCriteriaToRubric.ts b/teachertool/src/transforms/addCriteriaToRubric.ts index a61c8db77dab..e124a207ca35 100644 --- a/teachertool/src/transforms/addCriteriaToRubric.ts +++ b/teachertool/src/transforms/addCriteriaToRubric.ts @@ -1,11 +1,10 @@ import { stateAndDispatch } from "../state"; -import * as Actions from "../state/actions"; import { getCatalogCriteriaWithId } from "../state/helpers"; import { logDebug, logError } from "../services/loggingService"; import { CriteriaInstance, CriteriaParameterValue } from "../types/criteria"; import { nanoid } from "nanoid"; import { ErrorCode } from "../types/errorCode"; -import * as AutorunService from "../services/autorunService"; +import { setRubric } from "./setRubric"; export function addCriteriaToRubric(catalogCriteriaIds: string[]) { const { state: teacherTool, dispatch } = stateAndDispatch(); @@ -45,8 +44,7 @@ export function addCriteriaToRubric(catalogCriteriaIds: string[]) { newRubric.criteria.push(criteriaInstance); } - dispatch(Actions.setRubric(newRubric)); - AutorunService.poke(); + 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 6d372e393304..34c5ed681e99 100644 --- a/teachertool/src/transforms/removeCriteriaFromRubric.ts +++ b/teachertool/src/transforms/removeCriteriaFromRubric.ts @@ -1,8 +1,7 @@ import { stateAndDispatch } from "../state"; -import * as Actions from "../state/actions"; import { logDebug } from "../services/loggingService"; import { CriteriaInstance } from "../types/criteria"; -import * as AutorunService from "../services/autorunService"; +import { setRubric } from "./setRubric"; export function removeCriteriaFromRubric(instance: CriteriaInstance) { const { state: teacherTool, dispatch } = stateAndDispatch(); @@ -14,8 +13,7 @@ export function removeCriteriaFromRubric(instance: CriteriaInstance) { criteria: teacherTool.rubric.criteria.filter(c => c.instanceId !== instance.instanceId), }; - dispatch(Actions.setRubric(newRubric)); - AutorunService.poke(); + setRubric(newRubric); pxt.tickEvent("teachertool.removecriteria", { catalogCriteriaId: instance.catalogCriteriaId }); } diff --git a/teachertool/src/transforms/setRubricName.ts b/teachertool/src/transforms/setRubricName.ts index 6d43a74f4fe3..c18bf9e0387d 100644 --- a/teachertool/src/transforms/setRubricName.ts +++ b/teachertool/src/transforms/setRubricName.ts @@ -1,8 +1,8 @@ import { stateAndDispatch } from "../state"; -import * as Actions from "../state/actions"; +import { setRubric } from "./setRubric"; export function setRubricName(name: string) { - const { state: teacherTool, dispatch } = stateAndDispatch(); + const { state: teacherTool } = stateAndDispatch(); const oldName = teacherTool.rubric.name; @@ -14,5 +14,5 @@ export function setRubricName(name: string) { ...teacherTool.rubric, name, }; - dispatch(Actions.setRubric(newRubric)); + setRubric(newRubric); }