From d9e679df1a56b2f90f758adeb3bc0156761f4544 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Fri, 25 Oct 2024 14:25:00 -0700 Subject: [PATCH 01/12] Remove excess whitespace --- src/actions/navigation.js | 4 ++-- src/actions/types.js | 1 - src/components/narrativeEditor/useDatasetFetch.js | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/actions/navigation.js b/src/actions/navigation.js index 04a43bd71..ac7e33134 100644 --- a/src/actions/navigation.js +++ b/src/actions/navigation.js @@ -90,8 +90,8 @@ export const changePage = ({ const oldState = getState(); /* set some defaults */ - if (!path) path = window.location.pathname; - if (!query) query = queryString.parse(window.location.search); + if (!path) path = window.location.pathname; + if (!query) query = queryString.parse(window.location.search); /* some booleans */ const pathHasChanged = oldState.general.pathname !== path; diff --git a/src/actions/types.js b/src/actions/types.js index fb59b92fe..a6ac00c80 100644 --- a/src/actions/types.js +++ b/src/actions/types.js @@ -63,4 +63,3 @@ export const TOGGLE_SHOW_ALL_BRANCH_LABELS = "TOGGLE_SHOW_ALL_BRANCH_LABELS"; export const TOGGLE_MOBILE_DISPLAY = "TOGGLE_MOBILE_DISPLAY"; export const SELECT_NODE = "SELECT_NODE"; export const DESELECT_NODE = "DESELECT_NODE"; - diff --git a/src/components/narrativeEditor/useDatasetFetch.js b/src/components/narrativeEditor/useDatasetFetch.js index c85de38c4..f6577265c 100644 --- a/src/components/narrativeEditor/useDatasetFetch.js +++ b/src/components/narrativeEditor/useDatasetFetch.js @@ -118,4 +118,3 @@ async function fetchDatasetAndSidecars(name, dataset, dispatchDatasetResponses) console.error("Programming error within fetchDatasetAndSidecars", err); } } - From 7599aa7a502e71b19237c69ba0610ba61f89c663 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 28 Oct 2024 12:38:18 -0700 Subject: [PATCH 02/12] Fetch and load measurements sidecar JSON upfront Fetch and load the measurements sidecar JSON upfront in order to pave the way for simplifying the merging of controls and URL params. This will be especially useful once measurements data can add controls to the main tree, e.g. colorings. Subsequent commits will clean up the Redux state and actions now that we can assume that the measurements data have been loaded. Will also need to fix the bug of URL param `m_collection` no longer being respected on page load. --- src/actions/loadData.js | 41 +++++-------- src/actions/measurements.js | 60 +++++++++++-------- src/actions/navigation.js | 2 + src/actions/recomputeReduxState.js | 11 ++-- .../narrativeEditor/examineNarrative.js | 1 + src/reducers/measurements.js | 11 +--- 6 files changed, 63 insertions(+), 63 deletions(-) diff --git a/src/actions/loadData.js b/src/actions/loadData.js index c16e43781..4b81391ca 100644 --- a/src/actions/loadData.js +++ b/src/actions/loadData.js @@ -4,7 +4,6 @@ import { getServerAddress } from "../util/globals"; import { goTo404 } from "./navigation"; import { createStateFromQueryOrJSONs, createTreeTooState, getNarrativePageFromQuery } from "./recomputeReduxState"; import { loadFrequencies } from "./frequencies"; -import { parseMeasurementsJSON, loadMeasurements } from "./measurements"; import { fetchJSON, fetchWithErrorHandling } from "../util/serverInteraction"; import { warningNotification, errorNotification } from "./notifications"; import { parseMarkdownNarrativeFile } from "../util/parseNarrative"; @@ -12,7 +11,6 @@ import { NoContentError, FetchError} from "../util/exceptions"; import { parseMarkdown } from "../util/parseMarkdown"; import { updateColorByWithRootSequenceData } from "../actions/colors"; import { explodeTree } from "./tree"; -import { togglePanelDisplay } from "./panelDisplay"; export function getDatasetNamesFromUrl(url) { let secondTreeUrl; @@ -121,6 +119,7 @@ function narrativeFetchingErrorNotification(err) { */ async function dispatchCleanStart(dispatch, main, second, query, narrativeBlocks) { const json = await main.main; + const measurementsData = main.measurements ? (await main.measurements) : undefined; const secondTreeDataset = second ? (await second.main) : undefined; const pathnameShouldBe = second ? `${main.pathname}:${second.pathname}` : main.pathname; dispatch({ @@ -128,6 +127,7 @@ async function dispatchCleanStart(dispatch, main, second, query, narrativeBlocks pathnameShouldBe: narrativeBlocks ? undefined : pathnameShouldBe, ...createStateFromQueryOrJSONs({ json, + measurementsData, secondTreeDataset, query, narrativeBlocks, @@ -280,7 +280,19 @@ Dataset.prototype.fetchMain = function fetchMain() { } return res; }) - .then((res) => res.json()); + .then((res) => res.json()) + .then((json) => { + if (json.meta.panels && json.meta.panels.includes("measurements") && !this.measurements) { + /** + * Fetch measurements and store the resulting promise. + * Avoid the browser's default unhandled promise rejection logging and + * just resolve to an Error object that will be handled appropriately in loadMeasurements. + */ + this.measurements = fetchJSON(this.apiCalls.measurements) + .catch((reason) => Promise.resolve(reason)); + } + return json; + }); }; Dataset.prototype.fetchSidecars = async function fetchSidecars() { /** @@ -304,12 +316,6 @@ Dataset.prototype.fetchSidecars = async function fetchSidecars() { this.rootSequence = fetchJSON(this.apiCalls.rootSequence) .catch((reason) => Promise.resolve(reason)) } - - if (mainJson.meta.panels && mainJson.meta.panels.includes("measurements") && !this.measurements) { - this.measurements = fetchJSON(this.apiCalls.measurements) - .then((json) => parseMeasurementsJSON(json)) - .catch((reason) => Promise.resolve(reason)) - } }; Dataset.prototype.loadSidecars = function loadSidecars(dispatch) { /* Helper function to load (dispatch) the visualisation of sidecar files. @@ -346,23 +352,6 @@ Dataset.prototype.loadSidecars = function loadSidecars(dispatch) { dispatch(warningNotification({message: "Failed to parse root sequence JSON"})); }) } - if (this.measurements) { - this.measurements - .then((data) => { - if (data instanceof Error) throw data; - return data - }) - .then((data) => dispatch(loadMeasurements(data))) - .catch((err) => { - const errorMessage = `Failed to ${err instanceof FetchError ? 'fetch' : 'parse'} measurements collections`; - console.error(errorMessage, err.message); - dispatch(warningNotification({message: errorMessage})); - // Hide measurements panel - dispatch(togglePanelDisplay("measurements")); - // Save error message to display if user toggles panel again - dispatch({ type: types.UPDATE_MEASUREMENTS_ERROR, data: errorMessage }); - }); - } }; Dataset.prototype.fetchAvailable = async function fetchAvailable() { this.available = fetchJSON(this.apiCalls.getAvailable); diff --git a/src/actions/measurements.js b/src/actions/measurements.js index b4b7c365f..8c6bb316f 100644 --- a/src/actions/measurements.js +++ b/src/actions/measurements.js @@ -1,12 +1,13 @@ import { cloneDeep, pick } from "lodash"; import { measurementIdSymbol } from "../util/globals"; import { defaultMeasurementsControlState } from "../reducers/controls"; +import { getDefaultMeasurementsState } from "../reducers/measurements"; +import { warningNotification } from "./notifications"; import { APPLY_MEASUREMENTS_FILTER, CHANGE_MEASUREMENTS_COLLECTION, CHANGE_MEASUREMENTS_DISPLAY, CHANGE_MEASUREMENTS_GROUP_BY, - LOAD_MEASUREMENTS, TOGGLE_MEASUREMENTS_OVERALL_MEAN, TOGGLE_MEASUREMENTS_THRESHOLD, } from "./types"; @@ -171,7 +172,7 @@ const getCollectionDisplayControls = (controls, collection) => { return newControls; }; -export const parseMeasurementsJSON = (json) => { +const parseMeasurementsJSON = (json) => { const collections = json["collections"]; if (!collections || collections.length === 0) { throw new Error("Measurements JSON does not have collections"); @@ -263,34 +264,45 @@ export const parseMeasurementsJSON = (json) => { ); }); - return {collections, defaultCollection: json["default_collection"]}; -}; - -export const loadMeasurements = ({collections, defaultCollection}) => (dispatch, getState) => { - const { tree, controls } = getState(); - if (!tree.loaded) { - throw new Error("tree not loaded"); - } - const collectionKeys = collections.map((collection) => collection.key); - let defaultCollectionKey = defaultCollection; + let defaultCollectionKey = json["default_collection"]; if (!collectionKeys.includes(defaultCollectionKey)) { defaultCollectionKey = collectionKeys[0]; } - - // Get the collection to display to set up default controls - const collectionToDisplay = getCollectionToDisplay(collections, controls.measurementsCollectionKey, defaultCollectionKey); - const newControls = getCollectionDisplayControls(controls, collectionToDisplay); - const queryParams = createMeasurementsQueryFromControls(newControls, collectionToDisplay, defaultCollectionKey); - - dispatch({ - type: LOAD_MEASUREMENTS, + const collectionToDisplay = collections.filter((collection) => collection.key === defaultCollectionKey)[0]; + return { + loaded: true, + error: undefined, defaultCollectionKey, collections, - collectionToDisplay, - controls: newControls, - queryParams - }); + collectionToDisplay + } +}; + +export const loadMeasurements = (measurementsData, dispatch) => { + let measurementState = getDefaultMeasurementsState(); + let warningMessage = ""; + if (measurementsData === undefined) { + // eslint-disable-next-line no-console + console.debug("No measurements JSON fetched"); + } else if (measurementsData instanceof Error) { + console.error(measurementsData); + warningMessage = "Failed to fetch measurements collections"; + } else { + try { + measurementState = { ...measurementState, ...parseMeasurementsJSON(measurementsData) }; + } catch (error) { + console.error(error); + warningMessage = "Failed to parse measurements collections"; + } + } + + if (warningMessage) { + measurementState.error = warningMessage; + dispatch(warningNotification({ message: warningMessage })); + } + + return measurementState; }; export const changeMeasurementsCollection = (newCollectionKey) => (dispatch, getState) => { diff --git a/src/actions/navigation.js b/src/actions/navigation.js index ac7e33134..06aae3dc1 100644 --- a/src/actions/navigation.js +++ b/src/actions/navigation.js @@ -40,11 +40,13 @@ const updateNarrativeDataset = async (dispatch, datasets, narrativeBlocks, path, const mainDataset = datasets[mainTreeName]; const secondDataset = datasets[secondTreeName]; const mainJson = await mainDataset.main; + const measurementsData = mainDataset.measurements ? (await mainDataset.measurements) : undefined; const secondJson = secondDataset ? (await secondDataset.main) : false; dispatch({ type: URL_QUERY_CHANGE_WITH_COMPUTED_STATE, ...createStateFromQueryOrJSONs({ json: mainJson, + measurementsData, secondTreeDataset: secondJson, mainTreeName, secondTreeName, diff --git a/src/actions/recomputeReduxState.js b/src/actions/recomputeReduxState.js index 588e5dc62..d886d5436 100644 --- a/src/actions/recomputeReduxState.js +++ b/src/actions/recomputeReduxState.js @@ -7,7 +7,6 @@ import { getIdxMatchingLabel, calculateVisiblityAndBranchThickness } from "../ut import { constructVisibleTipLookupBetweenTrees } from "../util/treeTangleHelpers"; import { getDefaultControlsState, shouldDisplayTemporalConfidence } from "../reducers/controls"; import { getDefaultFrequenciesState } from "../reducers/frequencies"; -import { getDefaultMeasurementsState } from "../reducers/measurements"; import { countTraitsAcrossTree, calcTotalTipsInTree, gatherTraitNames } from "../util/treeCountingHelpers"; import { calcEntropyInView } from "../util/entropy"; import { treeJsonToState } from "../util/treeJsonProcessing"; @@ -23,7 +22,7 @@ import { getTraitFromNode, getDivFromNode, collectGenotypeStates } from "../util import { collectAvailableTipLabelOptions } from "../components/controls/choose-tip-label"; import { hasMultipleGridPanels } from "./panelDisplay"; import { strainSymbolUrlString } from "../middleware/changeURL"; -import { createMeasurementsControlsFromQuery, getCollectionDefaultControls, getCollectionToDisplay } from "./measurements"; +import { createMeasurementsControlsFromQuery, getCollectionDefaultControls, getCollectionToDisplay, loadMeasurements } from "./measurements"; export const doesColorByHaveConfidence = (controlsState, colorBy) => controlsState.coloringsPresentOnTreeWithConfidence.has(colorBy); @@ -858,6 +857,7 @@ export const getNarrativePageFromQuery = (query, narrative) => { export const createStateFromQueryOrJSONs = ({ json = false, /* raw json data - completely nuke existing redux state */ + measurementsData = false, /* raw measurements json data or error, only used when main json is provided */ secondTreeDataset = false, oldState = false, /* existing redux state (instead of jsons) */ narrativeBlocks = false, /* if in a narrative this argument is set */ @@ -875,8 +875,8 @@ export const createStateFromQueryOrJSONs = ({ entropy = entropyCreateState(json.meta.genome_annotations); /* ensure default frequencies state */ frequencies = getDefaultFrequenciesState(); - /* ensure default measurements state */ - measurements = getDefaultMeasurementsState(); + /* Load measurements if available, otherwise ensure default measurements state */ + measurements = loadMeasurements(measurementsData, dispatch); /* new tree state(s) */ tree = treeJsonToState(json.tree); castIncorrectTypes(metadata, tree); @@ -891,6 +891,9 @@ export const createStateFromQueryOrJSONs = ({ controls = getDefaultControlsState(); controls = modifyControlsStateViaTree(controls, tree, treeToo, metadata.colorings); controls = modifyStateViaMetadata(controls, metadata, entropy.genomeMap); + if (measurements.loaded) { + controls = {...controls, ...getCollectionDefaultControls(measurements.collectionToDisplay)}; + } } else if (oldState) { /* creating deep copies avoids references to (nested) objects remaining the same which can affect props comparisons. Due to the size of some of the state, we only do this selectively */ diff --git a/src/components/narrativeEditor/examineNarrative.js b/src/components/narrativeEditor/examineNarrative.js index 8ee0b7bc8..9a20ab9fe 100644 --- a/src/components/narrativeEditor/examineNarrative.js +++ b/src/components/narrativeEditor/examineNarrative.js @@ -202,6 +202,7 @@ const ExamineNarrative = ({narrative, datasetResponses, setDisplayNarrative}) => preserveCache: true, // specific for the narrative debugger ...createStateFromQueryOrJSONs({ json: await narrative.datasets[datasetNames[0]].main, + measurementsData: narrative.datasets[datasetNames[0]].measurements ? (await narrative.datasets[datasetNames[0]].measurements) : undefined, secondTreeDataset: datasetNames[1] ? await narrative.datasets[datasetNames[1]].main : undefined, query: n ? {n} : {}, // query is things like n=3 to load a specific page narrativeBlocks: narrative.blocks, diff --git a/src/reducers/measurements.js b/src/reducers/measurements.js index b0e2af395..3fc440b01 100644 --- a/src/reducers/measurements.js +++ b/src/reducers/measurements.js @@ -1,6 +1,6 @@ import { CHANGE_MEASUREMENTS_COLLECTION, - LOAD_MEASUREMENTS, + CLEAN_START, UPDATE_MEASUREMENTS_ERROR, URL_QUERY_CHANGE_WITH_COMPUTED_STATE } from "../actions/types"; @@ -15,16 +15,9 @@ export const getDefaultMeasurementsState = () => ({ const measurements = (state = getDefaultMeasurementsState(), action) => { switch (action.type) { + case CLEAN_START: // fallthrough case URL_QUERY_CHANGE_WITH_COMPUTED_STATE: return { ...action.measurements }; - case LOAD_MEASUREMENTS: - return { - ...state, - loaded: true, - defaultCollectionKey: action.defaultCollectionKey, - collections: action.collections, - collectionToDisplay: action.collectionToDisplay - }; case CHANGE_MEASUREMENTS_COLLECTION: return { ...state, From 047de263a7b6ed1779e2d56c8aecbc251ff8b636 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 28 Oct 2024 12:38:50 -0700 Subject: [PATCH 03/12] Remove unused `LOAD_MEASUREMENTS` action --- src/actions/types.js | 1 - src/middleware/changeURL.js | 1 - src/reducers/controls.ts | 1 - 3 files changed, 3 deletions(-) diff --git a/src/actions/types.js b/src/actions/types.js index a6ac00c80..0c336bce4 100644 --- a/src/actions/types.js +++ b/src/actions/types.js @@ -51,7 +51,6 @@ export const CACHE_JSONS = "CACHE_JSONS"; export const SET_ROOT_SEQUENCE = "SET_ROOT_SEQUENCE"; export const CHANGE_TIP_LABEL_KEY = "CHANGE_TIP_LABEL_KEY"; export const CHANGE_EXPLODE_ATTR = "CHANGE_EXPLODE_ATTR"; -export const LOAD_MEASUREMENTS = "LOAD_MEASUREMENTS"; export const CHANGE_MEASUREMENTS_COLLECTION = "CHANGE_MEASUREMENTS_COLLECTION"; export const CHANGE_MEASUREMENTS_GROUP_BY = "CHANGE_MEASUREMENTS_GROUP_BY"; export const TOGGLE_MEASUREMENTS_THRESHOLD = "TOGGLE_MEASUREMENTS_THRESHOLD"; diff --git a/src/middleware/changeURL.js b/src/middleware/changeURL.js index a57511c7f..da22f195f 100644 --- a/src/middleware/changeURL.js +++ b/src/middleware/changeURL.js @@ -223,7 +223,6 @@ export const changeURLMiddleware = (store) => (next) => (action) => { } break; } - case types.LOAD_MEASUREMENTS: // fallthrough case types.CHANGE_MEASUREMENTS_COLLECTION: // fallthrough case types.APPLY_MEASUREMENTS_FILTER: query = removeInvalidMeasurementsFilterQuery(query, action.queryParams) diff --git a/src/reducers/controls.ts b/src/reducers/controls.ts index 5fc96bb77..5c3160b61 100644 --- a/src/reducers/controls.ts +++ b/src/reducers/controls.ts @@ -386,7 +386,6 @@ const Controls = (state: ControlsState = getDefaultControlsState(), action): Con } return state; } - case types.LOAD_MEASUREMENTS: // fallthrough case types.CHANGE_MEASUREMENTS_COLLECTION: // fallthrough case types.CHANGE_MEASUREMENTS_DISPLAY: // fallthrough case types.CHANGE_MEASUREMENTS_GROUP_BY: // fallthrough From a5bc80fdf2bee144472f5f0e51a85e86e421825c Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Tue, 29 Oct 2024 13:23:18 -0700 Subject: [PATCH 04/12] Remove unused `UPDATE_MEASUREMENTS_ERROR` action --- src/actions/types.js | 1 - src/reducers/measurements.js | 7 ------- 2 files changed, 8 deletions(-) diff --git a/src/actions/types.js b/src/actions/types.js index 0c336bce4..719bd2275 100644 --- a/src/actions/types.js +++ b/src/actions/types.js @@ -57,7 +57,6 @@ export const TOGGLE_MEASUREMENTS_THRESHOLD = "TOGGLE_MEASUREMENTS_THRESHOLD"; export const TOGGLE_MEASUREMENTS_OVERALL_MEAN = "TOGGLE_MEASUREMENTS_OVERALL_MEAN"; export const CHANGE_MEASUREMENTS_DISPLAY = "CHANGE_MEASUREMENTS_DISPLAY"; export const APPLY_MEASUREMENTS_FILTER = "APPLY_MEASUREMENTS_FILTER"; -export const UPDATE_MEASUREMENTS_ERROR = "UPDATE_MEASUREMENTS_ERROR"; export const TOGGLE_SHOW_ALL_BRANCH_LABELS = "TOGGLE_SHOW_ALL_BRANCH_LABELS"; export const TOGGLE_MOBILE_DISPLAY = "TOGGLE_MOBILE_DISPLAY"; export const SELECT_NODE = "SELECT_NODE"; diff --git a/src/reducers/measurements.js b/src/reducers/measurements.js index 3fc440b01..f7805e951 100644 --- a/src/reducers/measurements.js +++ b/src/reducers/measurements.js @@ -1,7 +1,6 @@ import { CHANGE_MEASUREMENTS_COLLECTION, CLEAN_START, - UPDATE_MEASUREMENTS_ERROR, URL_QUERY_CHANGE_WITH_COMPUTED_STATE } from "../actions/types"; @@ -24,12 +23,6 @@ const measurements = (state = getDefaultMeasurementsState(), action) => { loaded: true, collectionToDisplay: action.collectionToDisplay }; - case UPDATE_MEASUREMENTS_ERROR: - return { - ...state, - loaded: true, - error: action.data - }; default: return state; } From 0f67361cbd7f39598240f148df2dc22de802a293 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Tue, 29 Oct 2024 09:47:08 -0700 Subject: [PATCH 05/12] Remove `controls.measurementsCollectionKey` Redux state This is no longer required since we load the measurements JSON upfront and we can compare the URL param `m_collection` to the loaded collection. --- src/actions/measurements.js | 23 +++++++---------------- src/reducers/controls.ts | 3 --- 2 files changed, 7 insertions(+), 19 deletions(-) diff --git a/src/actions/measurements.js b/src/actions/measurements.js index 8c6bb316f..6b14b0267 100644 --- a/src/actions/measurements.js +++ b/src/actions/measurements.js @@ -92,7 +92,6 @@ function getCollectionDefaultControl(controlKey, collection) { } break; } - case 'measurementsCollectionKey': // fallthrough case 'measurementsFilters': { // eslint-disable-next-line no-console console.debug(`Skipping control key ${controlKey} because it does not have default controls`); @@ -113,7 +112,6 @@ function getCollectionDefaultControl(controlKey, collection) { export function getCollectionDefaultControls(collection) { const defaultControls = {...defaultMeasurementsControlState}; if (Object.keys(collection).length) { - defaultControls.measurementsCollectionKey = collection.key; for (const [key, value] of Object.entries(defaultControls)) { const collectionDefault = getCollectionDefaultControl(key, collection); defaultControls[key] = collectionDefault !== undefined ? collectionDefault : value; @@ -135,7 +133,6 @@ export function getCollectionDefaultControls(collection) { const getCollectionDisplayControls = (controls, collection) => { // Copy current control options for measurements const newControls = cloneDeep(pick(controls, Object.keys(defaultMeasurementsControlState))); - newControls.measurementsCollectionKey = collection.key; // Checks the current group by is available as a grouping in collection // If it doesn't exist, set to undefined so it will get filled in with collection's default if (!collection.groupings.has(newControls.measurementsGroupBy)) { @@ -365,7 +362,7 @@ export const removeAllFieldFilters = (field) => (dispatch, getState) => { dispatch({ type: APPLY_MEASUREMENTS_FILTER, controls: { measurementsFilters }, - queryParams: createMeasurementsQueryFromControls({measurementsFilters}, measurements.collectionToDisplay) + queryParams: createMeasurementsQueryFromControls({measurementsFilters}, measurements.collectionToDisplay, measurements.defaultCollectionKey) }); }; @@ -391,7 +388,7 @@ export const toggleOverallMean = () => (dispatch, getState) => { dispatch({ type: TOGGLE_MEASUREMENTS_OVERALL_MEAN, controls: newControls, - queryParams: createMeasurementsQueryFromControls(newControls, measurements.collectionToDisplay) + queryParams: createMeasurementsQueryFromControls(newControls, measurements.collectionToDisplay, measurements.defaultCollectionKey) }); } @@ -415,7 +412,7 @@ export const changeMeasurementsDisplay = (newDisplay) => (dispatch, getState) => dispatch({ type: CHANGE_MEASUREMENTS_DISPLAY, controls: newControls, - queryParams: createMeasurementsQueryFromControls(newControls, measurements.collectionToDisplay) + queryParams: createMeasurementsQueryFromControls(newControls, measurements.collectionToDisplay, measurements.defaultCollectionKey) }); } @@ -432,7 +429,6 @@ export const changeMeasurementsGroupBy = (newGroupBy) => (dispatch, getState) => } const controlToQueryParamMap = { - measurementsCollectionKey: "m_collection", measurementsDisplay: "m_display", measurementsGroupBy: "m_groupBy", measurementsShowOverallMean: "m_overallMean", @@ -450,8 +446,10 @@ export function removeInvalidMeasurementsFilterQuery(query, newQueryParams) { return newQuery } -function createMeasurementsQueryFromControls(measurementControls, collection, defaultCollectionKey) { - const newQuery = {}; +export function createMeasurementsQueryFromControls(measurementControls, collection, defaultCollectionKey) { + const newQuery = { + m_collection: collection.key === defaultCollectionKey ? "" : collection.key + }; for (const [controlKey, controlValue] of Object.entries(measurementControls)) { let queryKey = controlToQueryParamMap[controlKey]; const collectionDefault = getCollectionDefaultControl(controlKey, collection); @@ -461,13 +459,6 @@ function createMeasurementsQueryFromControls(measurementControls, collection, de newQuery[queryKey] = ""; } else { switch(controlKey) { - case "measurementsCollectionKey": - if (controlValue !== defaultCollectionKey) { - newQuery[queryKey] = controlValue; - } else { - newQuery[queryKey] = ""; - } - break; case "measurementsDisplay": // fallthrough case "measurementsGroupBy": newQuery[queryKey] = controlValue; diff --git a/src/reducers/controls.ts b/src/reducers/controls.ts index 5c3160b61..1f0b2c979 100644 --- a/src/reducers/controls.ts +++ b/src/reducers/controls.ts @@ -44,7 +44,6 @@ export interface BasicControlsState { } export interface MeasurementsControlState { - measurementsCollectionKey: string | undefined, measurementsGroupBy: string | undefined, measurementsDisplay: string | undefined, measurementsShowOverallMean: boolean | undefined, @@ -134,7 +133,6 @@ export const getDefaultControlsState = () => { showOnlyPanels: false, showTransmissionLines: true, normalizeFrequencies: true, - measurementsCollectionKey: undefined, measurementsGroupBy: undefined, measurementsDisplay: undefined, measurementsShowOverallMean: undefined, @@ -153,7 +151,6 @@ export const getDefaultControlsState = () => { * differentiate the clean slate vs the added URL params. */ export const defaultMeasurementsControlState: MeasurementsControlState = { - measurementsCollectionKey: undefined, measurementsGroupBy: undefined, measurementsDisplay: "mean", measurementsShowOverallMean: true, From 0861269886822368434fdf53c438a29db11372b3 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 28 Oct 2024 13:49:18 -0700 Subject: [PATCH 06/12] parseMeasurementsJSON: deep clone `json.collections` Since the json values are cached for narratives, `parseMeasurementsJSON` should _not_ edit the json values directly. This commit fixes the TypeError that occurs when switching back to previously loaded dataset in narratives. --- src/actions/measurements.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/actions/measurements.js b/src/actions/measurements.js index 6b14b0267..63dfc372b 100644 --- a/src/actions/measurements.js +++ b/src/actions/measurements.js @@ -170,7 +170,8 @@ const getCollectionDisplayControls = (controls, collection) => { }; const parseMeasurementsJSON = (json) => { - const collections = json["collections"]; + // Avoid editing the original json values since they are cached for narratives + const collections = cloneDeep(json["collections"]); if (!collections || collections.length === 0) { throw new Error("Measurements JSON does not have collections"); } From a2431ae05df669b906acd0caaecf0ccde71567b3 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 28 Oct 2024 12:51:09 -0700 Subject: [PATCH 07/12] measurements: display error even if data not loaded Allow the measurements panel to display any errors even if the measurements data failed to load. This will allow users to see errors in the measurement's panel even if the warning notification goes away. --- src/components/measurements/index.js | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/src/components/measurements/index.js b/src/components/measurements/index.js index 39f5256b5..d7c8c300d 100644 --- a/src/components/measurements/index.js +++ b/src/components/measurements/index.js @@ -343,20 +343,19 @@ const Measurements = ({height, width, showLegend}) => { return ( - {measurementsLoaded && - (measurementsError ? - -

- {measurementsError} -

- : - - ) + {measurementsError ? + +

+ {measurementsError} +

+ : + (measurementsLoaded && + ) } ); From 947eda6d57553a005f8ec62f4a715f5ab0366b37 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Mon, 28 Oct 2024 13:24:55 -0700 Subject: [PATCH 08/12] Add `combineMeasurementsControlsAndQuery` Consolidate measurements controls and query param handling in `combineMeasurementsControlsAndQuery`. Now that we load measurements JSON upfront, we can use the collection's data to validate the query params on initial load of the page. Handling all measurements controls and query params in one place to make it easier to ensure they are kept in sync. --- src/actions/measurements.js | 109 +++++++++++++++++++++-------- src/actions/recomputeReduxState.js | 34 +++------ 2 files changed, 89 insertions(+), 54 deletions(-) diff --git a/src/actions/measurements.js b/src/actions/measurements.js index 63dfc372b..6aa078843 100644 --- a/src/actions/measurements.js +++ b/src/actions/measurements.js @@ -25,7 +25,7 @@ import { * @param {string} defaultKey * @returns {Object} */ -export const getCollectionToDisplay = (collections, collectionKey, defaultKey) => { +const getCollectionToDisplay = (collections, collectionKey, defaultKey) => { const defaultCollection = collections.filter((collection) => collection.key === defaultKey)[0]; if (!collectionKey) return defaultCollection; const potentialCollections = collections.filter((collection) => collection.key === collectionKey); @@ -109,7 +109,7 @@ function getCollectionDefaultControl(controlKey, collection) { * @param {Object} collection * @returns {MeasurementsControlState} */ -export function getCollectionDefaultControls(collection) { +function getCollectionDefaultControls(collection) { const defaultControls = {...defaultMeasurementsControlState}; if (Object.keys(collection).length) { for (const [key, value] of Object.entries(defaultControls)) { @@ -447,7 +447,7 @@ export function removeInvalidMeasurementsFilterQuery(query, newQueryParams) { return newQuery } -export function createMeasurementsQueryFromControls(measurementControls, collection, defaultCollectionKey) { +function createMeasurementsQueryFromControls(measurementControls, collection, defaultCollectionKey) { const newQuery = { m_collection: collection.key === defaultCollectionKey ? "" : collection.key }; @@ -497,49 +497,100 @@ export function createMeasurementsQueryFromControls(measurementControls, collect return newQuery; } -export function createMeasurementsControlsFromQuery(query){ - const newState = {}; +/** + * Parses the current collection's controls from measurements and updates them + * with valid query parameters. + * + * In cases where the query param is invalid, the query param is removed from the + * returned query object. + * @param {Object} measurements + * @param {Object} query + * @returns {Object} + */ +export const combineMeasurementsControlsAndQuery = (measurements, query) => { + const updatedQuery = cloneDeep(query); + const collectionKeys = measurements.collections.map((collection) => collection.key); + // Remove m_collection query if it's invalid or the default collection key + if (!collectionKeys.includes(updatedQuery.m_collection) || + updatedQuery.m_collection === measurements.defaultCollectionKey) { + delete updatedQuery.m_collection; + } + // Parse collection's default controls + const collectionKey = updatedQuery.m_collection || measurements.defaultCollectionKey; + const collectionToDisplay = getCollectionToDisplay(measurements.collections, collectionKey, measurements.defaultCollectionKey) + const collectionControls = getCollectionDefaultControls(collectionToDisplay); + const collectionGroupings = Array.from(collectionToDisplay.groupings.keys()); + // Modify controls via query for (const [controlKey, queryKey] of Object.entries(controlToQueryParamMap)) { - const queryValue = query[queryKey]; + const queryValue = updatedQuery[queryKey]; if (queryValue === undefined) continue; - let expectedValues = []; - let conversionFn = () => null; + let newControlState = undefined; switch(queryKey) { case "m_display": - expectedValues = ["mean", "raw"]; - conversionFn = () => queryValue; + if (queryValue === "mean" || queryValue === "raw") { + newControlState = queryValue; + } break; - case "m_collection": // fallthrough case "m_groupBy": - // Accept any value here because we cannot validate the query before - // the measurements JSON is loaded - expectedValues = [queryValue]; - conversionFn = () => queryValue; + // Verify value is a valid grouping of collection + if (collectionGroupings.includes(queryValue)) { + newControlState = queryValue; + } + break; + case "m_overallMean": + if (queryValue === "show" || queryValue === "hide") { + newControlState = queryValue === "show"; + } break; - case "m_overallMean": // fallthrough case "m_threshold": - expectedValues = ["show", "hide"]; - conversionFn = () => queryValue === "show"; + if (collectionToDisplay.thresholds && + (queryValue === "show" || queryValue === "hide")) { + newControlState = queryValue === "show"; + } break; + default: + console.error(`Ignoring unsupported query ${queryKey}`); } - if(expectedValues.includes(queryValue)) { - newState[controlKey] = conversionFn(); - } else { - console.error(`Ignoring invalid query param ${queryKey}=${queryValue}, value should be one of ${expectedValues}`); + // Remove query if it's invalid or the same as the collection's default controls + if (newControlState === undefined || newControlState === collectionControls[controlKey]) { + delete updatedQuery[queryKey]; + continue; } + collectionControls[controlKey] = newControlState } - // Accept any value here because we cannot validate the query before the measurements JSON is loaded - for (const filterKey of Object.keys(query).filter((c) => c.startsWith(filterQueryPrefix))) { + // Special handling of the filter query since these can be arbitrary query keys `mf_*` + for (const filterKey of Object.keys(updatedQuery).filter((c) => c.startsWith(filterQueryPrefix))) { + // Remove and ignore query for invalid fields const field = filterKey.replace(filterQueryPrefix, ''); - const filterValues = Array.isArray(query[filterKey]) ? query[filterKey] : [query[filterKey]]; - const measurementsFilters = {...newState.measurementsFilters}; + if (!collectionToDisplay.filters.has(field)) { + delete updatedQuery[filterKey]; + continue; + } + + // Remove and ignore query for invalid field values + const collectionFieldValues = collectionToDisplay.filters.get(field).values; + const filterValues = Array.isArray(updatedQuery[filterKey]) ? updatedQuery[filterKey] : [updatedQuery[filterKey]]; + const validFilterValues = filterValues.filter((value) => collectionFieldValues.has(value)); + if (!validFilterValues.length) { + delete updatedQuery[filterKey]; + continue; + } + + // Set field filter controls and query to the valid filter values + updatedQuery[filterKey] = validFilterValues; + const measurementsFilters = {...collectionControls.measurementsFilters}; measurementsFilters[field] = new Map(measurementsFilters[field]); - for (const value of filterValues) { + for (const value of validFilterValues) { measurementsFilters[field].set(value, {active: true}); } - newState.measurementsFilters = measurementsFilters; + collectionControls.measurementsFilters = measurementsFilters; + + } + return { + collectionToDisplay, + collectionControls, + updatedQuery } - return newState; } diff --git a/src/actions/recomputeReduxState.js b/src/actions/recomputeReduxState.js index d886d5436..6dcc52aa6 100644 --- a/src/actions/recomputeReduxState.js +++ b/src/actions/recomputeReduxState.js @@ -22,7 +22,7 @@ import { getTraitFromNode, getDivFromNode, collectGenotypeStates } from "../util import { collectAvailableTipLabelOptions } from "../components/controls/choose-tip-label"; import { hasMultipleGridPanels } from "./panelDisplay"; import { strainSymbolUrlString } from "../middleware/changeURL"; -import { createMeasurementsControlsFromQuery, getCollectionDefaultControls, getCollectionToDisplay, loadMeasurements } from "./measurements"; +import { combineMeasurementsControlsAndQuery, loadMeasurements } from "./measurements"; export const doesColorByHaveConfidence = (controlsState, colorBy) => controlsState.coloringsPresentOnTreeWithConfidence.has(colorBy); @@ -207,9 +207,6 @@ const modifyStateViaURLQuery = (state, query) => { if (query.scatterX) state.scatterVariables.x = query.scatterX; if (query.scatterY) state.scatterVariables.y = query.scatterY; - /* Process query params for measurements panel. These all start with `m_` or `mf_` prefix to avoid conflicts */ - state = {...state, ...createMeasurementsControlsFromQuery(query)} - return state; function _validDate(dateNum, absoluteDateMinNumeric, absoluteDateMaxNumeric) { return !(dateNum===undefined || dateNum > absoluteDateMaxNumeric || dateNum < absoluteDateMinNumeric); @@ -891,9 +888,6 @@ export const createStateFromQueryOrJSONs = ({ controls = getDefaultControlsState(); controls = modifyControlsStateViaTree(controls, tree, treeToo, metadata.colorings); controls = modifyStateViaMetadata(controls, metadata, entropy.genomeMap); - if (measurements.loaded) { - controls = {...controls, ...getCollectionDefaultControls(measurements.collectionToDisplay)}; - } } else if (oldState) { /* creating deep copies avoids references to (nested) objects remaining the same which can affect props comparisons. Due to the size of some of the state, we only do this selectively */ @@ -906,12 +900,6 @@ export const createStateFromQueryOrJSONs = ({ measurements = {...oldState.measurements}; controls = restoreQueryableStateToDefaults(controls); controls = modifyStateViaMetadata(controls, metadata, entropy.genomeMap); - /* If available, reset to the default collection and the collection's default controls - so that narrative queries are respected between slides */ - if (measurements.loaded) { - measurements.collectionToDisplay = getCollectionToDisplay(measurements.collections, "", measurements.defaultCollectionKey) - controls = {...controls, ...getCollectionDefaultControls(measurements.collectionToDisplay)}; - } } /* For the creation of state, we want to parse out URL query parameters @@ -925,22 +913,18 @@ export const createStateFromQueryOrJSONs = ({ narrativeSlideIdx = getNarrativePageFromQuery(query, narrative); /* replace the query with the information which can guide the view */ query = queryString.parse(narrative[narrativeSlideIdx].query); - /** - * Special case where narrative includes query param for new measurements collection `m_collection` - * We need to reset the measurements and controls to the new collection's defaults before - * processing the remaining query params - */ - if (query.m_collection && measurements.loaded) { - const newCollectionToDisplay = getCollectionToDisplay(measurements.collections, query.m_collection, measurements.defaultCollectionKey); - measurements.collectionToDisplay = newCollectionToDisplay; - controls = {...controls, ...getCollectionDefaultControls(measurements.collectionToDisplay)}; - // Delete `m_collection` so there's no chance of things getting mixed up when processing remaining query params - delete query.m_collection; - } } controls = modifyStateViaURLQuery(controls, query); + /* Special handling of measurements controls and query params */ + if (measurements.loaded) { + const { collectionToDisplay, collectionControls, updatedQuery} = combineMeasurementsControlsAndQuery(measurements, query); + measurements.collectionToDisplay = collectionToDisplay; + controls = {...controls, ...collectionControls}; + query = updatedQuery; + } + /* certain narrative slides prescribe the main panel to simply render narrative-provided markdown content */ if (narrativeBlocks && narrative[narrativeSlideIdx].mainDisplayMarkdown) { controls.panelsToDisplay = ["MainDisplayMarkdown"]; From 6c2308a5eef7871fe973186179ec2910340e5060 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Tue, 29 Oct 2024 17:00:24 -0700 Subject: [PATCH 09/12] Hide measurements panel if loading failed --- src/actions/recomputeReduxState.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/actions/recomputeReduxState.js b/src/actions/recomputeReduxState.js index 6dcc52aa6..bb6020e06 100644 --- a/src/actions/recomputeReduxState.js +++ b/src/actions/recomputeReduxState.js @@ -923,6 +923,11 @@ export const createStateFromQueryOrJSONs = ({ measurements.collectionToDisplay = collectionToDisplay; controls = {...controls, ...collectionControls}; query = updatedQuery; + } else { + // Hide measurements panel if loading failed + controls.panelsToDisplay = controls.panelsToDisplay.filter((panel) => panel !== "measurements"); + controls.canTogglePanelLayout = hasMultipleGridPanels(controls.panelsToDisplay); + controls.panelLayout = controls.canTogglePanelLayout ? controls.panelLayout : "full"; } /* certain narrative slides prescribe the main panel to simply render narrative-provided markdown content */ From cab837e3103a99e4e58482a7b110366bbe2bc504 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Tue, 5 Nov 2024 11:40:45 -0800 Subject: [PATCH 10/12] Measurements: Switch conditionals for rendering Switch the conditionals for rendering so that the Measurements panel always shows the appropriate content. This adds a default error message for the panel in case the `measurementsLoaded` and `measurementsError` states are ever out of sync. Based on feedback from @jameshadfield --- src/components/measurements/index.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/components/measurements/index.js b/src/components/measurements/index.js index d7c8c300d..9d6fe6dee 100644 --- a/src/components/measurements/index.js +++ b/src/components/measurements/index.js @@ -343,19 +343,19 @@ const Measurements = ({height, width, showLegend}) => { return ( - {measurementsError ? + {measurementsLoaded ? + :

- {measurementsError} + {measurementsError || + "Failed to fetch/load measurements due to unknown error"}

- : - (measurementsLoaded && - ) + } ); From 0069e09eb66a8a3654b660d881bcb39dac765cd1 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Tue, 5 Nov 2024 11:49:23 -0800 Subject: [PATCH 11/12] loadMeasurements: fix bug for warning notifications Fix bug where the warning notification for measurements JSON was being fired even when the dataset did not request the measurements panel. Based on feedback from @jameshadfield --- src/actions/measurements.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/actions/measurements.js b/src/actions/measurements.js index 6aa078843..7e43a8800 100644 --- a/src/actions/measurements.js +++ b/src/actions/measurements.js @@ -279,11 +279,13 @@ const parseMeasurementsJSON = (json) => { export const loadMeasurements = (measurementsData, dispatch) => { let measurementState = getDefaultMeasurementsState(); + /* Just return default state there are no measurements data to load */ + if (!measurementsData) { + return measurementState + } + let warningMessage = ""; - if (measurementsData === undefined) { - // eslint-disable-next-line no-console - console.debug("No measurements JSON fetched"); - } else if (measurementsData instanceof Error) { + if (measurementsData instanceof Error) { console.error(measurementsData); warningMessage = "Failed to fetch measurements collections"; } else { From c03d336afb0c44969b7281047f5caa8fb54154f4 Mon Sep 17 00:00:00 2001 From: Jover Lee Date: Tue, 5 Nov 2024 12:30:49 -0800 Subject: [PATCH 12/12] Remove measurements query params if loading failed --- src/actions/recomputeReduxState.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/actions/recomputeReduxState.js b/src/actions/recomputeReduxState.js index bb6020e06..41917ed23 100644 --- a/src/actions/recomputeReduxState.js +++ b/src/actions/recomputeReduxState.js @@ -928,6 +928,11 @@ export const createStateFromQueryOrJSONs = ({ controls.panelsToDisplay = controls.panelsToDisplay.filter((panel) => panel !== "measurements"); controls.canTogglePanelLayout = hasMultipleGridPanels(controls.panelsToDisplay); controls.panelLayout = controls.canTogglePanelLayout ? controls.panelLayout : "full"; + // Remove all measurements query params which start with `m_` or `mf_` + query = Object.fromEntries( + Object.entries(query) + .filter(([key, _]) => !(key.startsWith("m_") || key.startsWith("mf_"))) + ); } /* certain narrative slides prescribe the main panel to simply render narrative-provided markdown content */