From a4d4b7b946f1224777e0b51d96e8fd6233ead508 Mon Sep 17 00:00:00 2001 From: khahani Date: Sat, 6 Jun 2026 21:40:34 +0400 Subject: [PATCH 1/2] show loading while fetching glossary --- source/Table.js | 20 ++++++++- source/__tests__/Table.test.js | 75 ++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/source/Table.js b/source/Table.js index fe024f72..febd029b 100644 --- a/source/Table.js +++ b/source/Table.js @@ -69,8 +69,9 @@ export default class Table extends Component { } async handleTable(file) { - const { user } = this.props; - + // The glossary download is started at app launch and runs in parallel. We + // need it to compile, so here we wait for it to finish if it hasn't yet. + let waitingForGlossarySwal = false; try { let shouldFetch = true; let serverVersion = null; @@ -89,6 +90,17 @@ export default class Table extends Component { } if (shouldFetch) { + // The glossary isn't ready yet; tell the scientist we're waiting on it. + Swal.fire({ + title: "Glossary …", + allowOutsideClick: false, + allowEscapeKey: false, + showConfirmButton: false, + willOpen: () => { + Swal.showLoading(null); + }, + }); + waitingForGlossarySwal = true; // Fetch by explicit version so the CDN returns the just-published // glossary (new version = new URL = cache miss), never a stale copy. // If the probe failed, serverVersion is null → falls back to current. @@ -96,9 +108,13 @@ export default class Table extends Component { initGlossary(data); } } catch (err) { + if (waitingForGlossarySwal) Swal.close(); console.error("Failed to refresh glossary:", err); return; } + // Close the "Glossary …" status before handing off to the resource/compile + // flow, which manages its own status dialog. + if (waitingForGlossarySwal) Swal.close(); let resolvedResources; diff --git a/source/__tests__/Table.test.js b/source/__tests__/Table.test.js index dccdd6ac..0b90ce39 100644 --- a/source/__tests__/Table.test.js +++ b/source/__tests__/Table.test.js @@ -1,6 +1,8 @@ import React from "react"; import { render } from "@testing-library/react"; import Table from "../Table"; +import Swal from "sweetalert2"; +import { preprocessExperimentFile } from "../../threshold/preprocess/main"; jest.mock("sweetalert2", () => ({ fire: jest.fn(), @@ -221,3 +223,76 @@ describe("Table.handleTable", () => { expect(initGlossary).toHaveBeenCalledWith(mockGlossaryData); }); }); + +describe("Table.handleTable glossary loading dialog", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + const glossaryFireCalls = () => + Swal.fire.mock.calls.filter( + ([opts]) => opts && opts.title === "Glossary …", + ); + + it("shows the 'Glossary …' status while downloading and closes it before handing off", async () => { + const { fetchGlossaryData, fetchGlossaryVersion } = require("../components/glossaryApi"); + const { getGlossaryVersion } = require("../../threshold/parameters/glossaryRegistry"); + fetchGlossaryVersion.mockResolvedValue({ version: "2.0" }); + getGlossaryVersion.mockReturnValue(null); + fetchGlossaryData.mockResolvedValue(mockGlossaryData); + + const ref = React.createRef(); + render(); + + await ref.current.handleTable(new File(["a,b"], "exp.csv")); + + // The loading dialog is opened exactly once... + expect(glossaryFireCalls()).toHaveLength(1); + // ...before the download starts... + const fireOrder = Swal.fire.mock.invocationCallOrder[0]; + const fetchOrder = fetchGlossaryData.mock.invocationCallOrder[0]; + expect(fireOrder).toBeLessThan(fetchOrder); + // ...and is closed before preprocessing takes over its own dialog. + expect(Swal.close).toHaveBeenCalledTimes(1); + const closeOrder = Swal.close.mock.invocationCallOrder[0]; + const preprocessOrder = preprocessExperimentFile.mock.invocationCallOrder[0]; + expect(closeOrder).toBeLessThan(preprocessOrder); + }); + + it("does not show the 'Glossary …' status when the cached version is current", async () => { + const { fetchGlossaryVersion } = require("../components/glossaryApi"); + const { getGlossaryVersion } = require("../../threshold/parameters/glossaryRegistry"); + fetchGlossaryVersion.mockResolvedValue({ version: "2.0" }); + getGlossaryVersion.mockReturnValue("2.0"); + + const ref = React.createRef(); + render(
); + + await ref.current.handleTable(new File(["a,b"], "exp.csv")); + + expect(glossaryFireCalls()).toHaveLength(0); + // Nothing was opened, so nothing needs closing for the glossary step. + expect(Swal.close).not.toHaveBeenCalled(); + }); + + it("closes the 'Glossary …' status when the download fails", async () => { + const { fetchGlossaryData, fetchGlossaryVersion } = require("../components/glossaryApi"); + const { getGlossaryVersion } = require("../../threshold/parameters/glossaryRegistry"); + fetchGlossaryVersion.mockResolvedValue({ version: "2.0" }); + getGlossaryVersion.mockReturnValue(null); + fetchGlossaryData.mockRejectedValue(new Error("network down")); + const consoleError = jest.spyOn(console, "error").mockImplementation(() => {}); + + const ref = React.createRef(); + render(
); + + await ref.current.handleTable(new File(["a,b"], "exp.csv")); + + expect(glossaryFireCalls()).toHaveLength(1); + // The error path closes the dialog instead of leaving it spinning forever. + expect(Swal.close).toHaveBeenCalledTimes(1); + expect(preprocessExperimentFile).not.toHaveBeenCalled(); + + consoleError.mockRestore(); + }); +}); From d3ebde0beb1c7d012a5e62eedb2df9d4355f618a Mon Sep 17 00:00:00 2001 From: khahani Date: Mon, 8 Jun 2026 13:50:35 +0400 Subject: [PATCH 2/2] Fetch glossary on file-select, not on compiler page load --- source/App.js | 18 +-- source/Running.js | 1 - source/Table.js | 64 +++++------ source/__tests__/App.test.js | 112 ------------------- source/__tests__/Table.test.js | 40 +++---- source/__tests__/prolificIntegration.test.js | 6 + source/components/prolificIntegration.js | 6 +- threshold | 2 +- 8 files changed, 62 insertions(+), 187 deletions(-) delete mode 100644 source/__tests__/App.test.js diff --git a/source/App.js b/source/App.js index 857c811f..f1379dd1 100644 --- a/source/App.js +++ b/source/App.js @@ -35,8 +35,7 @@ import "./css/App.scss"; import { signInAnonymously } from "firebase/auth"; import { getSoundProfileStatement } from "./components/firebase_soundProfile"; import { captureError } from "./sentry"; -import { fetchGlossaryData } from "./components/glossaryApi"; -import { initGlossary } from "../threshold/parameters/glossaryRegistry"; +import { getGlossaryFull } from "../threshold/parameters/glossaryRegistry"; // Utility function to create empty resources object from constants const createEmptyResourcesObject = () => { @@ -102,7 +101,6 @@ export default class App extends Component { profileStatement: "Loading ...", isCompiledFromArchiveBool: false, archivedZip: null, - glossaryData: null, compileWarnings: [], }; @@ -145,13 +143,6 @@ export default class App extends Component { } async componentDidMount() { - fetchGlossaryData() - .then((data) => { - initGlossary(data); - this.setState({ glossaryData: data }); - }) - .catch((error) => console.warn("Failed to fetch glossary data:", error)); - // get the actual changes from GitHub try { const websiteGitHubRepo = await fetch( @@ -707,12 +698,9 @@ export default class App extends Component { isCompiledFromArchiveBool, archivedZip, resourcesLoaded, - glossaryData, compileWarnings, } = this.state; - if (glossaryData === null) return null; - const steps = []; const viewingPreviousExperiment = @@ -742,7 +730,6 @@ export default class App extends Component { isCompiledFromArchiveBool={isCompiledFromArchiveBool} archivedZip={archivedZip} resourcesLoaded={resourcesLoaded} - glossaryData={glossaryData} />, ); else @@ -765,7 +752,6 @@ export default class App extends Component { isCompiledFromArchiveBool={isCompiledFromArchiveBool} archivedZip={archivedZip} resourcesLoaded={resourcesLoaded} - glossaryData={glossaryData} compileWarnings={compileWarnings} />, ); @@ -776,7 +762,7 @@ export default class App extends Component { }> )} diff --git a/source/Running.js b/source/Running.js index 3d0084e5..f5b5ef60 100644 --- a/source/Running.js +++ b/source/Running.js @@ -637,7 +637,6 @@ export default class Running extends Component { incompatibleCompletionCode, abortedCompletionCode, prolificToken, - this.props.glossaryData, ); if (result?.status === "UNPUBLISHED" && result.id) { diff --git a/source/Table.js b/source/Table.js index febd029b..dc3aa200 100644 --- a/source/Table.js +++ b/source/Table.js @@ -69,52 +69,46 @@ export default class Table extends Component { } async handleTable(file) { - // The glossary download is started at app launch and runs in parallel. We - // need it to compile, so here we wait for it to finish if it hasn't yet. - let waitingForGlossarySwal = false; + // The glossary is fetched lazily on first compile (no longer at app launch). + // handleDrop has already opened a "Compiling ..." dialog before calling us; + // we relabel that same dialog for each phase instead of firing/closing our + // own, so the modal stays open continuously — closing it would leave a blank + // screen through the rest of the compile. + let shouldFetch = true; + let serverVersion = null; try { - let shouldFetch = true; - let serverVersion = null; - try { - ({ version: serverVersion } = await fetchGlossaryVersion()); - const cachedVersion = getGlossaryVersion(); - if ( - serverVersion !== null && - cachedVersion !== null && - serverVersion === cachedVersion - ) { - shouldFetch = false; - } - } catch { - // fall through to full fetch + ({ version: serverVersion } = await fetchGlossaryVersion()); + const cachedVersion = getGlossaryVersion(); + if ( + serverVersion !== null && + cachedVersion !== null && + serverVersion === cachedVersion + ) { + shouldFetch = false; } + } catch { + // fall through to full fetch + } - if (shouldFetch) { - // The glossary isn't ready yet; tell the scientist we're waiting on it. - Swal.fire({ - title: "Glossary …", - allowOutsideClick: false, - allowEscapeKey: false, - showConfirmButton: false, - willOpen: () => { - Swal.showLoading(null); - }, - }); - waitingForGlossarySwal = true; + if (shouldFetch) { + // The glossary isn't ready yet; tell the scientist we're waiting on it. + manuallySetSwalTitle("Loading glossary …"); + Swal.showLoading(null); + try { // Fetch by explicit version so the CDN returns the just-published // glossary (new version = new URL = cache miss), never a stale copy. // If the probe failed, serverVersion is null → falls back to current. const data = await fetchGlossaryData(serverVersion); initGlossary(data); + } catch (err) { + Swal.close(); + console.error("Failed to refresh glossary:", err); + return; } - } catch (err) { - if (waitingForGlossarySwal) Swal.close(); - console.error("Failed to refresh glossary:", err); - return; } - // Close the "Glossary …" status before handing off to the resource/compile + // Restore the compiling status before handing off to the resource/compile // flow, which manages its own status dialog. - if (waitingForGlossarySwal) Swal.close(); + manuallySetSwalTitle("Compiling ..."); let resolvedResources; diff --git a/source/__tests__/App.test.js b/source/__tests__/App.test.js deleted file mode 100644 index 59f824f8..00000000 --- a/source/__tests__/App.test.js +++ /dev/null @@ -1,112 +0,0 @@ -import React from "react"; -import { render, waitFor } from "@testing-library/react"; -import App from "../App"; - -jest.mock("firebase/database", () => ({ - set: jest.fn(), - ref: jest.fn(), - get: jest.fn().mockResolvedValue({ val: () => ({ count: 0 }) }), -})); - -jest.mock("@firebase/util", () => ({ - uuidv4: jest.fn(() => "test-uuid"), -})); - -jest.mock("sweetalert2", () => ({ - fire: jest.fn(), - showLoading: jest.fn(), - close: jest.fn(), -})); - -jest.mock("../Step", () => () => null); -jest.mock("../Glossary", () => ({ default: () => null })); -jest.mock("../StatusLines", () => () => null); - -jest.mock("../components/steps", () => ({ - allSteps: jest.fn(() => ["login", "table", "upload", "running"]), -})); - -jest.mock("../../threshold/preprocess/gitlabUtils", () => ({ - getCompatibilityRequirementsForProject: jest.fn(), - getExperimentStatus: jest.fn(), - getOriginalFileNameForProject: jest.fn(), - getRecruitmentServiceConfig: jest.fn(), - getDurationForProject: jest.fn(), - getProlificStudyId: jest.fn(), - User: jest.fn(() => ({})), - getCommonResourcesNames: jest.fn(), -})); - -jest.mock("../../threshold/preprocess/retry", () => ({ - getRetryDelayMs: jest.fn(() => 0), -})); - -jest.mock("../../threshold/preprocess/constants", () => ({ - resourcesFileTypes: ["fonts", "images"], -})); - -jest.mock("../components/firebase", () => ({ - auth: {}, - db: {}, -})); - -jest.mock("../components/prolificIntegration", () => ({ - getProlificAccount: jest.fn(), - getProlificStudySubmissions: jest.fn(), -})); - -jest.mock("../../threshold/components/compatibilityCheck", () => ({ - getCompatibilityRequirements: jest.fn(), -})); - -jest.mock("../../threshold/preprocess/global", () => ({ - compatibilityRequirements: { t: [] }, -})); - -jest.mock("firebase/auth", () => ({ - signInAnonymously: jest.fn().mockResolvedValue({}), -})); - -jest.mock("../components/firebase_soundProfile", () => ({ - getSoundProfileStatement: jest.fn(), -})); - -jest.mock("../sentry", () => ({ - captureError: jest.fn(), -})); - -jest.mock("../components/glossaryApi", () => ({ - fetchGlossaryData: jest.fn(), -})); - -jest.mock("../../threshold/parameters/glossaryRegistry", () => ({ - initGlossary: jest.fn(), -})); - -global.fetch = jest.fn().mockResolvedValue({ ok: false }); - -const mockGlossaryData = { - version: "1.0", - glossary: { param1: { name: "param1" } }, - glossaryFull: [], - superMatchingParams: ["param1"], -}; - -describe("App", () => { - beforeEach(() => { - jest.clearAllMocks(); - const { fetchGlossaryData } = require("../components/glossaryApi"); - fetchGlossaryData.mockResolvedValue(mockGlossaryData); - global.fetch.mockResolvedValue({ ok: false }); - }); - - it("calls initGlossary with fetched glossary data when fetchGlossaryData resolves", async () => { - const { initGlossary } = require("../../threshold/parameters/glossaryRegistry"); - - render(); - - await waitFor(() => { - expect(initGlossary).toHaveBeenCalledWith(mockGlossaryData); - }); - }); -}); diff --git a/source/__tests__/Table.test.js b/source/__tests__/Table.test.js index 0b90ce39..7e711d5d 100644 --- a/source/__tests__/Table.test.js +++ b/source/__tests__/Table.test.js @@ -229,14 +229,15 @@ describe("Table.handleTable glossary loading dialog", () => { jest.clearAllMocks(); }); - const glossaryFireCalls = () => - Swal.fire.mock.calls.filter( - ([opts]) => opts && opts.title === "Glossary …", - ); + const swalTitles = () => { + const { manuallySetSwalTitle } = require("../../threshold/preprocess/gitlabUtils"); + return manuallySetSwalTitle.mock.calls.map(([title]) => title); + }; - it("shows the 'Glossary …' status while downloading and closes it before handing off", async () => { + it("relabels the open dialog to 'Glossary …' while downloading, then restores 'Compiling ...' without closing it", async () => { const { fetchGlossaryData, fetchGlossaryVersion } = require("../components/glossaryApi"); const { getGlossaryVersion } = require("../../threshold/parameters/glossaryRegistry"); + const { manuallySetSwalTitle } = require("../../threshold/preprocess/gitlabUtils"); fetchGlossaryVersion.mockResolvedValue({ version: "2.0" }); getGlossaryVersion.mockReturnValue(null); fetchGlossaryData.mockResolvedValue(mockGlossaryData); @@ -246,20 +247,21 @@ describe("Table.handleTable glossary loading dialog", () => { await ref.current.handleTable(new File(["a,b"], "exp.csv")); - // The loading dialog is opened exactly once... - expect(glossaryFireCalls()).toHaveLength(1); + const titles = swalTitles(); + // The dialog opened by handleDrop is relabeled to show the glossary download... + expect(titles).toContain("Glossary …"); // ...before the download starts... - const fireOrder = Swal.fire.mock.invocationCallOrder[0]; + const glossaryTitleOrder = + manuallySetSwalTitle.mock.invocationCallOrder[titles.indexOf("Glossary …")]; const fetchOrder = fetchGlossaryData.mock.invocationCallOrder[0]; - expect(fireOrder).toBeLessThan(fetchOrder); - // ...and is closed before preprocessing takes over its own dialog. - expect(Swal.close).toHaveBeenCalledTimes(1); - const closeOrder = Swal.close.mock.invocationCallOrder[0]; - const preprocessOrder = preprocessExperimentFile.mock.invocationCallOrder[0]; - expect(closeOrder).toBeLessThan(preprocessOrder); + expect(glossaryTitleOrder).toBeLessThan(fetchOrder); + // ...and is restored to "Compiling ..." (never closed) before preprocessing. + expect(titles).toContain("Compiling ..."); + expect(Swal.close).not.toHaveBeenCalled(); + expect(preprocessExperimentFile).toHaveBeenCalledTimes(1); }); - it("does not show the 'Glossary …' status when the cached version is current", async () => { + it("does not relabel to 'Glossary …' when the cached version is current", async () => { const { fetchGlossaryVersion } = require("../components/glossaryApi"); const { getGlossaryVersion } = require("../../threshold/parameters/glossaryRegistry"); fetchGlossaryVersion.mockResolvedValue({ version: "2.0" }); @@ -270,12 +272,12 @@ describe("Table.handleTable glossary loading dialog", () => { await ref.current.handleTable(new File(["a,b"], "exp.csv")); - expect(glossaryFireCalls()).toHaveLength(0); - // Nothing was opened, so nothing needs closing for the glossary step. + // No download, so no glossary status; the shared dialog stays open (never closed). + expect(swalTitles()).not.toContain("Glossary …"); expect(Swal.close).not.toHaveBeenCalled(); }); - it("closes the 'Glossary …' status when the download fails", async () => { + it("closes the dialog when the glossary download fails", async () => { const { fetchGlossaryData, fetchGlossaryVersion } = require("../components/glossaryApi"); const { getGlossaryVersion } = require("../../threshold/parameters/glossaryRegistry"); fetchGlossaryVersion.mockResolvedValue({ version: "2.0" }); @@ -288,7 +290,7 @@ describe("Table.handleTable glossary loading dialog", () => { await ref.current.handleTable(new File(["a,b"], "exp.csv")); - expect(glossaryFireCalls()).toHaveLength(1); + expect(swalTitles()).toContain("Glossary …"); // The error path closes the dialog instead of leaving it spinning forever. expect(Swal.close).toHaveBeenCalledTimes(1); expect(preprocessExperimentFile).not.toHaveBeenCalled(); diff --git a/source/__tests__/prolificIntegration.test.js b/source/__tests__/prolificIntegration.test.js index 197124cf..8a1c9ee6 100644 --- a/source/__tests__/prolificIntegration.test.js +++ b/source/__tests__/prolificIntegration.test.js @@ -8,6 +8,11 @@ import { COMPLETION_CODE_ACTION, COMPLETION_CODE_TYPE, } from "../components/prolificConstants"; +import { getGlossary } from "../../threshold/parameters/glossaryRegistry"; + +jest.mock("../../threshold/parameters/glossaryRegistry", () => ({ + getGlossary: jest.fn(), +})); // Mock global fetch global.fetch = jest.fn(); @@ -33,6 +38,7 @@ describe("Prolific Integration - New Parameters", () => { _online2Description: { default: "Default Description" }, }, }; + getGlossary.mockReturnValue(mockGlossaryData.glossary); mockInternalName = "TestExperiment"; mockCompletionCode = "COMPLETED123"; mockIncompatibleCode = "INCOMPATIBLE456"; diff --git a/source/components/prolificIntegration.js b/source/components/prolificIntegration.js index ff91f977..b22e6e23 100644 --- a/source/components/prolificIntegration.js +++ b/source/components/prolificIntegration.js @@ -18,6 +18,7 @@ import { VR_HEADSET_USAGE_PROLIFIC_MAPPING, } from "./prolificConstants"; import { captureError } from "../sentry"; +import { getGlossary } from "../../threshold/parameters/glossaryRegistry"; const prolificStudySubmissionStatus = { RESERVED: "RESERVED", @@ -360,7 +361,6 @@ export const prolificCreateDraft = async ( incompatibleCompletionCode, abortedCompletionCode, token, - glossaryData, ) => { // const prolificStudyDraftApiUrl = "https://api.prolific.com/api/v1/studies/"; const prolificStudyDraftApiUrl = "/.netlify/functions/prolific/studies/"; @@ -516,7 +516,7 @@ export const prolificCreateDraft = async ( user.currentExperiment.titleOfStudy && user.currentExperiment.titleOfStudy !== "" ? user.currentExperiment.titleOfStudy - : glossaryData.glossary["_online1Title"].default, + : getGlossary()["_online1Title"].default, internal_name: user.currentExperiment._online1InternalName && user.currentExperiment._online1InternalName !== "" @@ -526,7 +526,7 @@ export const prolificCreateDraft = async ( user.currentExperiment.descriptionOfStudy && user.currentExperiment.descriptionOfStudy !== "" ? user.currentExperiment.descriptionOfStudy - : glossaryData.glossary["_online2Description"].default, + : getGlossary()["_online2Description"].default, external_study_url: user.currentExperiment.experimentUrl, prolific_id_option: "url_parameters", completion_option: "url", diff --git a/threshold b/threshold index 170596b8..c59827ab 160000 --- a/threshold +++ b/threshold @@ -1 +1 @@ -Subproject commit 170596b84269299d31b95d3c8698e6b260126c49 +Subproject commit c59827ab95adecfaac5f30bcb1455a097b79241b