From f830bc7710a96719d4fd692435ebb2bfea9efc95 Mon Sep 17 00:00:00 2001 From: Garrett Michael Flynn Date: Tue, 26 Mar 2024 11:19:18 -0700 Subject: [PATCH 1/4] Allow for validating in groups on a table. --- schemas/base-metadata.schema.ts | 7 +- schemas/json/base_metadata_schema.json | 4 +- src/renderer/src/stories/SimpleTable.js | 2 +- src/renderer/src/stories/Table.js | 178 +++++++++++------- .../pages/guided-mode/setup/GuidedSubjects.js | 12 +- src/renderer/src/validation/index.js | 3 + src/renderer/src/validation/validation.json | 5 +- src/renderer/src/validation/validation.ts | 5 + 8 files changed, 138 insertions(+), 78 deletions(-) diff --git a/schemas/base-metadata.schema.ts b/schemas/base-metadata.schema.ts index c8a16f336..1fe054fd3 100644 --- a/schemas/base-metadata.schema.ts +++ b/schemas/base-metadata.schema.ts @@ -64,11 +64,12 @@ export const preprocessMetadataSchema = (schema: any = baseMetadataSchema, globa M: 'Male', F: 'Female', U: 'Unknown', - O: 'Other' + O: 'Other', + XX: 'Hermaphrodite — C. elegans', + XO: 'Male — C. elegans' } - - subjectProps.species = { + subjectProps.species = { type: 'string', strict: false, description: 'The species of your subject.' diff --git a/schemas/json/base_metadata_schema.json b/schemas/json/base_metadata_schema.json index c180408a7..2e694d499 100644 --- a/schemas/json/base_metadata_schema.json +++ b/schemas/json/base_metadata_schema.json @@ -172,7 +172,9 @@ "M", "F", "U", - "O" + "O", + "XX", + "XO" ] }, "species": { diff --git a/src/renderer/src/stories/SimpleTable.js b/src/renderer/src/stories/SimpleTable.js index c5e7d8fdb..18487401f 100644 --- a/src/renderer/src/stories/SimpleTable.js +++ b/src/renderer/src/stories/SimpleTable.js @@ -508,7 +508,7 @@ export class SimpleTable extends LitElement { Object.keys(cols).map((k) => (cols[k] = "")); if (this.validateOnChange) Object.keys(cols).map((k) => { - const res = this.validateOnChange([k], { ...cols }, cols[k]); + const res = this.validateOnChange([k], { ...cols }, cols[k]); // NOTE: This is likely incorrect if (typeof res === "function") res(); }); diff --git a/src/renderer/src/stories/Table.js b/src/renderer/src/stories/Table.js index 201bb8827..2c2ce72c7 100644 --- a/src/renderer/src/stories/Table.js +++ b/src/renderer/src/stories/Table.js @@ -119,6 +119,7 @@ export class Table extends LitElement { onThrow, contextMenu, ignore, + groups // NOTE: All groups must be non-overlapping } = {}) { super(); this.schema = schema ?? {}; @@ -128,6 +129,7 @@ export class Table extends LitElement { this.validateEmptyCells = validateEmptyCells ?? true; this.contextMenu = contextMenu ?? {}; this.ignore = ignore ?? {}; + this.groups = groups ?? []; if (onThrow) this.onThrow = onThrow; if (onUpdate) this.onUpdate = onUpdate; @@ -228,6 +230,8 @@ export class Table extends LitElement { }); }); }; + + #info = {} updated() { const div = (this.shadowRoot ?? this).querySelector("div"); @@ -277,7 +281,8 @@ export class Table extends LitElement { const displayHeaders = [...colHeaders].map(header); const columns = colHeaders.map((k, i) => { - const info = { type: "text" }; + const info = this.#info[k] = { type: "text", stopPersonalUpdates: {}, stopGroupUpdates: {} }; + const colInfo = entries[k]; if (colInfo.unit) displayHeaders[i] = `${displayHeaders[i]} (${colInfo.unit})`; @@ -309,9 +314,10 @@ export class Table extends LitElement { const runThisValidator = async (value, row, prop) => { try { + const path = [row, k] const valid = this.validateOnChange ? await this.validateOnChange( - [k], + path, { ...this.data[this.getRowName(row)] }, // Validate on a copy of the parent value, info @@ -328,6 +334,14 @@ export class Table extends LitElement { const required = isRequired(k, this.#itemSchema); const validator = async function (value, callback) { + + + const row = this.row; + // if (info.stopPersonalUpdates[row]) { + // callback(true) + // return + // } + const validateEmptyCells = instanceThis.validateEmptyCells; const willValidate = validateEmptyCells === true || @@ -339,7 +353,7 @@ export class Table extends LitElement { if (!value && !willValidate) { instanceThis.#handleValidationResult( [], // Clear errors - this.row, + row, this.col ); callback(true); // Allow empty value @@ -347,18 +361,18 @@ export class Table extends LitElement { } if (value && k === instanceThis.keyColumn) { - if (value in instanceThis.data && instanceThis.data[value]?.[rowSymbol] !== this.row) { + if (value in instanceThis.data && instanceThis.data[value]?.[rowSymbol] !== row) { // Convert previously valid value to unresolved - const previousKey = instanceThis.getRowName(this.row); + const previousKey = instanceThis.getRowName(rrow); if (previousKey) { - unresolved[this.row] = instanceThis.data[previousKey]; + unresolved[row] = instanceThis.data[previousKey]; delete instanceThis.data[previousKey]; } // Throw error instanceThis.#handleValidationResult( [{ message: `${header(k)} already exists`, type: "error" }], - this.row, + row, this.col ); callback(false); @@ -366,20 +380,8 @@ export class Table extends LitElement { } } - if (name === "subject_id") { - if (v) { - if (Object.values(this.data).filter((s) => s.subject_id === v).length > 1) { - return [ - { - message: "Subject ID must be unique", - type: "error", - }, - ]; - } - } - } - - if (!(await runThisValidator(value, this.row, this.col))) { + + if (!(await runThisValidator(value, row, this.col))) { callback(false); return; } @@ -387,7 +389,7 @@ export class Table extends LitElement { if (!value && required) { instanceThis.#handleValidationResult( [{ message: `${header(k)} is a required property.`, type: "error" }], - this.row, + row, this.col ); callback(false); @@ -508,71 +510,109 @@ export class Table extends LitElement { if (menu) this.#root.appendChild(menu); // Move to style root let validated = 0; - const initialCellsToUpdate = data.reduce((acc, v) => acc + v.length, 0); + + const initialCellsToUpdate = data.reduce((acc, arr) => acc + arr.length, 0); table.addHook("afterValidate", (isValid, value, row, prop) => { - const isUserUpdate = initialCellsToUpdate <= validated; - let rowName = this.getRowName(row); + const header = typeof prop === "number" ? colHeaders[prop] : prop; + const info = this.#info[header]; - if (isUserUpdate) { - const header = typeof prop === "number" ? colHeaders[prop] : prop; + // Update other columns in the group - // NOTE: We would like to allow invalid values to mutate the results - // if (isValid) { - const isResolved = rowName in this.data; - let target = this.data; + const skipUpdate = info.stopPersonalUpdates[row] || info.stopGroupUpdates[row]; + + // Decrement counters + if (info.stopPersonalUpdates[row]) info.stopPersonalUpdates[row]--; + if (info.stopGroupUpdates[row]) info.stopGroupUpdates[row]-- - if (!isResolved) { - if (!this.keyColumn) - this.data[rowName] = {}; // Add new row to array - else { - rowName = row; - if (!unresolved[rowName]) unresolved[rowName] = {}; // Ensure row exists - target = unresolved; + if (!skipUpdate) { + + const isUserUpdate = initialCellsToUpdate <= validated; + + let rowName = this.getRowName(row); + + if (isUserUpdate) { + + // NOTE: We would like to allow invalid values to mutate the results + // if (isValid) { + const isResolved = rowName in this.data; + let target = this.data; + + if (!isResolved) { + if (!this.keyColumn) + this.data[rowName] = {}; // Add new row to array + else { + rowName = row; + if (!unresolved[rowName]) unresolved[rowName] = {}; // Ensure row exists + target = unresolved; + } } - } - value = this.#getValue(value, entries[header]); - - // Transfer data to object (if valid) - if (header === this.keyColumn) { - if (isValid && value && value !== rowName) { - const old = target[rowName] ?? {}; - this.data[value] = old; - delete target[rowName]; - delete unresolved[row]; - Object.defineProperty(this.data[value], rowSymbol, { value: row, configurable: true }); // Setting row tracker - this.revalidate([{ row, prop }]); + value = this.#getValue(value, entries[header]); + + // Transfer data to object (if valid) + if (header === this.keyColumn) { + if (isValid && value && value !== rowName) { + const old = target[rowName] ?? {}; + this.data[value] = old; + delete target[rowName]; + delete unresolved[row]; + Object.defineProperty(this.data[value], rowSymbol, { value: row, configurable: true }); // Setting row tracker + this.revalidate([{ row, prop }]); + } + } + + // Update data on passed object + else { + const globalValue = this.globals[header]; + + if (value == undefined || value === "") { + if (globalValue) { + value = target[rowName][header] = globalValue; + table.setDataAtCell(row, prop, value); + this.onOverride(header, value, rowName); + } + target[rowName][header] = undefined; + } else { + // Correct for expected arrays (copy-paste issue) + if (entries[header]?.type === "array") { + if (value && !Array.isArray(value)) value = value.split(",").map((v) => v.trim()); + } + + target[rowName][header] = value === globalValue ? undefined : value; + } } + + this.onUpdate(rowName, header, value); } - // Update data on passed object - else { - const globalValue = this.globals[header]; + validated++; - if (value == undefined || value === "") { - if (globalValue) { - value = target[rowName][header] = globalValue; - table.setDataAtCell(row, prop, value); - this.onOverride(header, value, rowName); - } - target[rowName][header] = undefined; - } else { - // Correct for expected arrays (copy-paste issue) - if (entries[header]?.type === "array") { - if (value && !Array.isArray(value)) value = value.split(",").map((v) => v.trim()); - } + // Check associated groups for validity + for (let group of this.groups) { + const table = this.table; + if (group.includes(header)) { + const otherGroup = group.filter((col) => col !== header); - target[rowName][header] = value === globalValue ? undefined : value; + info.stopPersonalUpdates[row] = otherGroup.length + + otherGroup.forEach((col) => { + const j = colHeaders.indexOf(col); + const value = table.getDataAtCell(row, j) + const depInfo = this.#info[col]; + const otherGroups = group.filter((c) => c !== col) + depInfo.stopGroupUpdates[row] = otherGroups.length; // Expecting this many updates from other members of the group + table.setDataAtCell(row, j, value); + }) + + return } } + - this.onUpdate(rowName, header, value); } - validated++; - if (typeof isValid === "function") isValid(); }); diff --git a/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js b/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js index 060e6a3f4..92759c4f3 100644 --- a/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js +++ b/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js @@ -152,6 +152,13 @@ export class GuidedSubjectsPage extends Page { keyColumn: "subject_id", validateEmptyCells: ["subject_id", "sessions"], contextMenu: contextMenuConfig, + groups: [ + [ + "sex", + 'species', + // 'age' + ], // Validate both when one is changed + ], onThrow: (message, type) => this.notify(message, type), onOverride: (name) => { this.notify(`${header(name)} has been overridden with a global value.`, "warning", 3000); @@ -159,8 +166,9 @@ export class GuidedSubjectsPage extends Page { onUpdate: () => { this.unsavedUpdates = "conversions"; }, - validateOnChange: (localPath, parent, v) => { + validateOnChange: function (localPath, parent, v ) { const name = localPath[localPath.length - 1]; + if (name === "sessions") { if (v?.length) return true; else { @@ -173,7 +181,7 @@ export class GuidedSubjectsPage extends Page { } } else { delete parent.sessions; // Delete sessions from parent copy - return validateOnChange(localPath, parent, ["Subject"], v); + return validateOnChange.call(this, name, parent, ["Subject", ...localPath.slice(0, -1)], v); } }, }); diff --git a/src/renderer/src/validation/index.js b/src/renderer/src/validation/index.js index e0d819e18..f0627c91c 100644 --- a/src/renderer/src/validation/index.js +++ b/src/renderer/src/validation/index.js @@ -54,9 +54,11 @@ export async function validateOnChange(name, parent, path, value) { functions = overridden && functions !== true ? false : matches; // Disable if not promised to exist—or use matches } + if (!functions || (Array.isArray(functions) && functions.length === 0)) return; // No validation for this field if (!Array.isArray(functions)) functions = [functions]; + // Validate multiple conditions. May be able to offload this to a single server-side call const results = functions.map((func) => { if (typeof func === "function") { @@ -77,6 +79,7 @@ export async function validateOnChange(name, parent, path, value) { }); const res = resolveAll(results, (arr) => { + arr = arr.map((v, i) => { const func = functions[i]; if (typeof func === "function") return v; diff --git a/src/renderer/src/validation/validation.json b/src/renderer/src/validation/validation.json index f2af26e36..420ea3887 100644 --- a/src/renderer/src/validation/validation.json +++ b/src/renderer/src/validation/validation.json @@ -4,7 +4,8 @@ "conversion_output_folder": false, "NWBFile": { - "*": "check_{*}", + "description": "check_description", + "experiment_description": "check_experiment_description", "identifier": false, "session_description": false, "lab": false, @@ -42,12 +43,12 @@ "Behavior": false, "Subject": { - "*": "check_subject_{*}", "sessions": false, "description": false, "genotype": false, "strain": false, "weight": false, + "sex": ["check_subject_sex"], "age__reference": false, "subject_id": "check_subject_id_exists", "species": ["check_subject_species_form", "check_subject_species_exists"], diff --git a/src/renderer/src/validation/validation.ts b/src/renderer/src/validation/validation.ts index 78462f2a3..37cbcdcce 100644 --- a/src/renderer/src/validation/validation.ts +++ b/src/renderer/src/validation/validation.ts @@ -66,6 +66,11 @@ const getTablePathInfo = (path: string[]) => { return { modality, table, row } } +// ----------------- Subject Validation ----------------- // + +// Validate the same in rows and tables +schema.Subject['*'] = { ...schema.Subject } + // ----------------- Joint Ophys and Ecephys Validation ----------------- // From f738565d343624a711d7d363cde05f08befe2de2 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 26 Mar 2024 18:21:43 +0000 Subject: [PATCH 2/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/renderer/src/stories/Table.js | 35 +++++++------------ .../pages/guided-mode/setup/GuidedSubjects.js | 6 ++-- src/renderer/src/validation/index.js | 3 -- 3 files changed, 16 insertions(+), 28 deletions(-) diff --git a/src/renderer/src/stories/Table.js b/src/renderer/src/stories/Table.js index 2c2ce72c7..2e7e2e6ac 100644 --- a/src/renderer/src/stories/Table.js +++ b/src/renderer/src/stories/Table.js @@ -119,7 +119,7 @@ export class Table extends LitElement { onThrow, contextMenu, ignore, - groups // NOTE: All groups must be non-overlapping + groups, // NOTE: All groups must be non-overlapping } = {}) { super(); this.schema = schema ?? {}; @@ -230,8 +230,8 @@ export class Table extends LitElement { }); }); }; - - #info = {} + + #info = {}; updated() { const div = (this.shadowRoot ?? this).querySelector("div"); @@ -281,8 +281,7 @@ export class Table extends LitElement { const displayHeaders = [...colHeaders].map(header); const columns = colHeaders.map((k, i) => { - const info = this.#info[k] = { type: "text", stopPersonalUpdates: {}, stopGroupUpdates: {} }; - + const info = (this.#info[k] = { type: "text", stopPersonalUpdates: {}, stopGroupUpdates: {} }); const colInfo = entries[k]; if (colInfo.unit) displayHeaders[i] = `${displayHeaders[i]} (${colInfo.unit})`; @@ -314,7 +313,7 @@ export class Table extends LitElement { const runThisValidator = async (value, row, prop) => { try { - const path = [row, k] + const path = [row, k]; const valid = this.validateOnChange ? await this.validateOnChange( path, @@ -334,14 +333,12 @@ export class Table extends LitElement { const required = isRequired(k, this.#itemSchema); const validator = async function (value, callback) { - - const row = this.row; // if (info.stopPersonalUpdates[row]) { // callback(true) // return // } - + const validateEmptyCells = instanceThis.validateEmptyCells; const willValidate = validateEmptyCells === true || @@ -380,7 +377,6 @@ export class Table extends LitElement { } } - if (!(await runThisValidator(value, row, this.col))) { callback(false); return; @@ -514,26 +510,23 @@ export class Table extends LitElement { const initialCellsToUpdate = data.reduce((acc, arr) => acc + arr.length, 0); table.addHook("afterValidate", (isValid, value, row, prop) => { - const header = typeof prop === "number" ? colHeaders[prop] : prop; const info = this.#info[header]; // Update other columns in the group const skipUpdate = info.stopPersonalUpdates[row] || info.stopGroupUpdates[row]; - + // Decrement counters if (info.stopPersonalUpdates[row]) info.stopPersonalUpdates[row]--; - if (info.stopGroupUpdates[row]) info.stopGroupUpdates[row]-- + if (info.stopGroupUpdates[row]) info.stopGroupUpdates[row]--; if (!skipUpdate) { - const isUserUpdate = initialCellsToUpdate <= validated; let rowName = this.getRowName(row); if (isUserUpdate) { - // NOTE: We would like to allow invalid values to mutate the results // if (isValid) { const isResolved = rowName in this.data; @@ -595,22 +588,20 @@ export class Table extends LitElement { if (group.includes(header)) { const otherGroup = group.filter((col) => col !== header); - info.stopPersonalUpdates[row] = otherGroup.length + info.stopPersonalUpdates[row] = otherGroup.length; otherGroup.forEach((col) => { const j = colHeaders.indexOf(col); - const value = table.getDataAtCell(row, j) + const value = table.getDataAtCell(row, j); const depInfo = this.#info[col]; - const otherGroups = group.filter((c) => c !== col) + const otherGroups = group.filter((c) => c !== col); depInfo.stopGroupUpdates[row] = otherGroups.length; // Expecting this many updates from other members of the group table.setDataAtCell(row, j, value); - }) + }); - return + return; } } - - } if (typeof isValid === "function") isValid(); diff --git a/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js b/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js index 92759c4f3..e4eb5491f 100644 --- a/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js +++ b/src/renderer/src/stories/pages/guided-mode/setup/GuidedSubjects.js @@ -154,8 +154,8 @@ export class GuidedSubjectsPage extends Page { contextMenu: contextMenuConfig, groups: [ [ - "sex", - 'species', + "sex", + "species", // 'age' ], // Validate both when one is changed ], @@ -166,7 +166,7 @@ export class GuidedSubjectsPage extends Page { onUpdate: () => { this.unsavedUpdates = "conversions"; }, - validateOnChange: function (localPath, parent, v ) { + validateOnChange: function (localPath, parent, v) { const name = localPath[localPath.length - 1]; if (name === "sessions") { diff --git a/src/renderer/src/validation/index.js b/src/renderer/src/validation/index.js index f0627c91c..e0d819e18 100644 --- a/src/renderer/src/validation/index.js +++ b/src/renderer/src/validation/index.js @@ -54,11 +54,9 @@ export async function validateOnChange(name, parent, path, value) { functions = overridden && functions !== true ? false : matches; // Disable if not promised to exist—or use matches } - if (!functions || (Array.isArray(functions) && functions.length === 0)) return; // No validation for this field if (!Array.isArray(functions)) functions = [functions]; - // Validate multiple conditions. May be able to offload this to a single server-side call const results = functions.map((func) => { if (typeof func === "function") { @@ -79,7 +77,6 @@ export async function validateOnChange(name, parent, path, value) { }); const res = resolveAll(results, (arr) => { - arr = arr.map((v, i) => { const func = functions[i]; if (typeof func === "function") return v; From 650c91637045ea648bc5f625d02a8ecf8136b5b7 Mon Sep 17 00:00:00 2001 From: Garrett Michael Flynn Date: Tue, 26 Mar 2024 12:07:41 -0700 Subject: [PATCH 3/4] Only group update interruption required --- src/renderer/src/stories/Table.js | 13 +++---------- src/renderer/src/validation/index.js | 1 + 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/renderer/src/stories/Table.js b/src/renderer/src/stories/Table.js index 2e7e2e6ac..4f391bff2 100644 --- a/src/renderer/src/stories/Table.js +++ b/src/renderer/src/stories/Table.js @@ -281,7 +281,7 @@ export class Table extends LitElement { const displayHeaders = [...colHeaders].map(header); const columns = colHeaders.map((k, i) => { - const info = (this.#info[k] = { type: "text", stopPersonalUpdates: {}, stopGroupUpdates: {} }); + const info = (this.#info[k] = { type: "text", stopGroupUpdates: {} }); const colInfo = entries[k]; if (colInfo.unit) displayHeaders[i] = `${displayHeaders[i]} (${colInfo.unit})`; @@ -334,10 +334,6 @@ export class Table extends LitElement { const validator = async function (value, callback) { const row = this.row; - // if (info.stopPersonalUpdates[row]) { - // callback(true) - // return - // } const validateEmptyCells = instanceThis.validateEmptyCells; const willValidate = @@ -515,11 +511,10 @@ export class Table extends LitElement { // Update other columns in the group - const skipUpdate = info.stopPersonalUpdates[row] || info.stopGroupUpdates[row]; + const skipUpdate = info.stopGroupUpdates[row]; // Decrement counters - if (info.stopPersonalUpdates[row]) info.stopPersonalUpdates[row]--; - if (info.stopGroupUpdates[row]) info.stopGroupUpdates[row]--; + if (skipUpdate) info.stopGroupUpdates[row]--; if (!skipUpdate) { const isUserUpdate = initialCellsToUpdate <= validated; @@ -588,8 +583,6 @@ export class Table extends LitElement { if (group.includes(header)) { const otherGroup = group.filter((col) => col !== header); - info.stopPersonalUpdates[row] = otherGroup.length; - otherGroup.forEach((col) => { const j = colHeaders.indexOf(col); const value = table.getDataAtCell(row, j); diff --git a/src/renderer/src/validation/index.js b/src/renderer/src/validation/index.js index e0d819e18..abe43354a 100644 --- a/src/renderer/src/validation/index.js +++ b/src/renderer/src/validation/index.js @@ -77,6 +77,7 @@ export async function validateOnChange(name, parent, path, value) { }); const res = resolveAll(results, (arr) => { + arr = arr.map((v, i) => { const func = functions[i]; if (typeof func === "function") return v; From de68525684469fc95b3a3a58771647737e9fadca Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 26 Mar 2024 19:08:06 +0000 Subject: [PATCH 4/4] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/renderer/src/validation/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/renderer/src/validation/index.js b/src/renderer/src/validation/index.js index abe43354a..e0d819e18 100644 --- a/src/renderer/src/validation/index.js +++ b/src/renderer/src/validation/index.js @@ -77,7 +77,6 @@ export async function validateOnChange(name, parent, path, value) { }); const res = resolveAll(results, (arr) => { - arr = arr.map((v, i) => { const func = functions[i]; if (typeof func === "function") return v;