From 9ef599902f56f2828fd91d7828eb23c331490ec2 Mon Sep 17 00:00:00 2001 From: abcampo-iry <261805581+abcampo-iry@users.noreply.github.com> Date: Fri, 20 Mar 2026 12:51:54 +0100 Subject: [PATCH 1/6] Track Scratch remix lifecycle in save state Handle Scratch remix start, success, and failure events in the shared save-state hook, and forward Scratch remix creation failures from the iframe host page. --- src/hooks/useScratchSaveState.js | 8 +++-- src/hooks/useScratchSaveState.test.js | 51 +++++++++++++-------------- src/scratch.jsx | 2 ++ 3 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/hooks/useScratchSaveState.js b/src/hooks/useScratchSaveState.js index 5e8d33a19..c7c0c389b 100644 --- a/src/hooks/useScratchSaveState.js +++ b/src/hooks/useScratchSaveState.js @@ -31,6 +31,7 @@ export const useScratchSaveState = ({ enabled = false } = {}) => { } const allowedOrigin = process.env.ASSETS_URL || window.location.origin; + const handleScratchMessage = (event) => { if (event.origin !== allowedOrigin) { return; @@ -38,10 +39,12 @@ export const useScratchSaveState = ({ enabled = false } = {}) => { switch (event.data?.type) { case "scratch-gui-saving-started": + case "scratch-gui-remixing-started": clearScratchSaveResetTimeout(); setScratchSaveState("saving"); break; case "scratch-gui-saving-succeeded": + case "scratch-gui-remixing-succeeded": clearScratchSaveResetTimeout(); setScratchSaveState("saved"); scratchSaveResetTimeoutRef.current = setTimeout(() => { @@ -50,6 +53,7 @@ export const useScratchSaveState = ({ enabled = false } = {}) => { }, SCRATCH_SAVE_RESET_DELAY_MS); break; case "scratch-gui-saving-failed": + case "scratch-gui-remixing-failed": resetScratchSaveState(); break; default: @@ -65,9 +69,9 @@ export const useScratchSaveState = ({ enabled = false } = {}) => { }; }, [enabled]); - const saveScratchProject = () => { + const saveScratchProject = ({ shouldRemixOnSave = false } = {}) => { postMessageToScratchIframe({ - type: "scratch-gui-save", + type: shouldRemixOnSave ? "scratch-gui-remix" : "scratch-gui-save", }); }; diff --git a/src/hooks/useScratchSaveState.test.js b/src/hooks/useScratchSaveState.test.js index fb83b6c50..032d94375 100644 --- a/src/hooks/useScratchSaveState.test.js +++ b/src/hooks/useScratchSaveState.test.js @@ -40,21 +40,27 @@ describe("useScratchSaveState", () => { process.env.ASSETS_URL = originalAssetsUrl; }); - test("returns idle save state by default", () => { + test("posts the scratch save command", () => { const { result } = renderHook(() => useScratchSaveState()); - assertScratchSaveState(result, "idle", "header.save", false); + act(() => { + result.current.saveScratchProject(); + }); + + expect(postMessageToScratchIframe).toHaveBeenCalledWith({ + type: "scratch-gui-save", + }); }); - test("posts the scratch save command", () => { + test("posts the scratch remix command on the first save", () => { const { result } = renderHook(() => useScratchSaveState()); act(() => { - result.current.saveScratchProject(); + result.current.saveScratchProject({ shouldRemixOnSave: true }); }); expect(postMessageToScratchIframe).toHaveBeenCalledWith({ - type: "scratch-gui-save", + type: "scratch-gui-remix", }); }); @@ -74,11 +80,21 @@ describe("useScratchSaveState", () => { assertScratchSaveState(result, "idle", "header.save", false); }); - test("resets to idle after a save failure", () => { + test("tracks remixing messages with the same save state lifecycle", () => { const { result } = renderHook(() => useScratchSaveState({ enabled: true })); - dispatchScratchMessage("scratch-gui-saving-started"); - dispatchScratchMessage("scratch-gui-saving-failed"); + dispatchScratchMessage("scratch-gui-remixing-started"); + assertScratchSaveState(result, "saving", "saveStatus.saving", true); + + dispatchScratchMessage("scratch-gui-remixing-succeeded"); + assertScratchSaveState(result, "saved", "saveStatus.saved", false); + }); + + test("resets to idle after a remix failure", () => { + const { result } = renderHook(() => useScratchSaveState({ enabled: true })); + + dispatchScratchMessage("scratch-gui-remixing-started"); + dispatchScratchMessage("scratch-gui-remixing-failed"); assertScratchSaveState(result, "idle", "header.save", false); }); @@ -93,23 +109,4 @@ describe("useScratchSaveState", () => { assertScratchSaveState(result, "idle", "header.save", false); }); - - test("resets and stops handling messages when disabled", () => { - const { result, rerender } = renderHook( - ({ enabled }) => useScratchSaveState({ enabled }), - { - initialProps: { enabled: true }, - }, - ); - - dispatchScratchMessage("scratch-gui-saving-started"); - expect(result.current.scratchSaveState).toBe("saving"); - - rerender({ enabled: false }); - - assertScratchSaveState(result, "idle", "header.save", false); - - dispatchScratchMessage("scratch-gui-saving-started"); - assertScratchSaveState(result, "idle", "header.save", false); - }); }); diff --git a/src/scratch.jsx b/src/scratch.jsx index 444a266df..f5482d087 100644 --- a/src/scratch.jsx +++ b/src/scratch.jsx @@ -53,6 +53,8 @@ const handleSavingSucceeded = () => const handleScratchGuiAlert = (alertType) => { if (alertType === "savingError") { postScratchGuiEvent("scratch-gui-saving-failed"); + } else if (alertType === "creatingError") { + postScratchGuiEvent("scratch-gui-remixing-failed"); } }; From d0cf68529cb85367e6591ae16ccc1ea13270db1c Mon Sep 17 00:00:00 2001 From: abcampo-iry <261805581+abcampo-iry@users.noreply.github.com> Date: Fri, 20 Mar 2026 12:52:03 +0100 Subject: [PATCH 2/6] Handle Scratch first-save remix in the parent Store the original Scratch iframe project identifier, update the parent project id when Scratch posts a remixed id, and use that state to remix only on the first save without reloading the iframe. --- .../Editor/Project/ScratchContainer.jsx | 31 ++++++- .../Editor/Project/ScratchContainer.test.js | 64 ++++++++++++-- .../ProjectBar/ScratchProjectBar.jsx | 13 ++- .../ProjectBar/ScratchProjectBar.test.js | 66 +++++++++++++- src/components/SaveButton/SaveButton.jsx | 39 ++++++++- src/components/SaveButton/SaveButton.test.js | 87 +++++++++++++++++++ .../WebComponentProject.integration.test.js | 34 +++++++- src/redux/EditorSlice.js | 13 +++ src/redux/EditorSlice.test.js | 35 ++++++++ src/redux/reducers/loadProjectReducers.js | 5 ++ .../reducers/loadProjectReducers.test.js | 1 + 11 files changed, 372 insertions(+), 16 deletions(-) diff --git a/src/components/Editor/Project/ScratchContainer.jsx b/src/components/Editor/Project/ScratchContainer.jsx index 4b3f909ef..716c4c559 100644 --- a/src/components/Editor/Project/ScratchContainer.jsx +++ b/src/components/Editor/Project/ScratchContainer.jsx @@ -1,16 +1,41 @@ -import React from "react"; -import { useSelector } from "react-redux"; +import React, { useEffect } from "react"; +import { useDispatch, useSelector } from "react-redux"; +import { applyScratchProjectIdentifierUpdate } from "../../../redux/EditorSlice"; export default function ScratchContainer() { + const dispatch = useDispatch(); const projectIdentifier = useSelector( (state) => state.editor.project.identifier, ); + const scratchIframeProjectIdentifier = useSelector( + (state) => state.editor.scratchIframeProjectIdentifier, + ); const scratchApiEndpoint = useSelector( (state) => state.editor.scratchApiEndpoint, ); + const iframeProjectIdentifier = + scratchIframeProjectIdentifier || projectIdentifier; + + useEffect(() => { + const allowedOrigin = process.env.ASSETS_URL || window.location.origin; + const handleScratchMessage = ({ origin, data }) => { + if (origin !== allowedOrigin) return; + if (data?.type !== "scratch-gui-project-id-updated") return; + if (!data.projectId) return; + + dispatch( + applyScratchProjectIdentifierUpdate({ + projectIdentifier: data.projectId, + }), + ); + }; + + window.addEventListener("message", handleScratchMessage); + return () => window.removeEventListener("message", handleScratchMessage); + }, [dispatch]); const queryParams = new URLSearchParams(); - queryParams.set("project_id", projectIdentifier); + queryParams.set("project_id", iframeProjectIdentifier); queryParams.set("api_url", scratchApiEndpoint); const iframeSrcUrl = `${ diff --git a/src/components/Editor/Project/ScratchContainer.test.js b/src/components/Editor/Project/ScratchContainer.test.js index 065c664eb..a51c596d4 100644 --- a/src/components/Editor/Project/ScratchContainer.test.js +++ b/src/components/Editor/Project/ScratchContainer.test.js @@ -1,8 +1,9 @@ -import configureStore from "redux-mock-store"; -import { render, screen } from "@testing-library/react"; +import { act, render, screen } from "@testing-library/react"; import React from "react"; import { Provider } from "react-redux"; +import { configureStore } from "@reduxjs/toolkit"; import ScratchContainer from "./ScratchContainer"; +import EditorReducer from "../../../redux/EditorSlice"; describe("ScratchContainer", () => { let originalAssetsUrl; @@ -17,13 +18,19 @@ describe("ScratchContainer", () => { }); test("renders iframe with src built from project_id and api_url", () => { - const mockStore = configureStore([]); - const store = mockStore({ - editor: { - project: { - identifier: "project-123", + const store = configureStore({ + reducer: { + editor: EditorReducer, + }, + preloadedState: { + editor: { + project: { + identifier: "project-123", + project_type: "code_editor_scratch", + }, + scratchIframeProjectIdentifier: "project-123", + scratchApiEndpoint: "https://api.example.com/v1", }, - scratchApiEndpoint: "https://api.example.com/v1", }, }); @@ -41,4 +48,45 @@ describe("ScratchContainer", () => { expect(url.searchParams.get("project_id")).toBe("project-123"); expect(url.searchParams.get("api_url")).toBe("https://api.example.com/v1"); }); + + test("updates the parent project identifier without changing the iframe project_id", () => { + const store = configureStore({ + reducer: { + editor: EditorReducer, + }, + preloadedState: { + editor: { + project: { + identifier: "project-123", + project_type: "code_editor_scratch", + }, + scratchIframeProjectIdentifier: "project-123", + scratchApiEndpoint: "https://api.example.com/v1", + }, + }, + }); + + render( + + + , + ); + + act(() => { + window.dispatchEvent( + new MessageEvent("message", { + origin: "https://example.com", + data: { + type: "scratch-gui-project-id-updated", + projectId: "project-456", + }, + }), + ); + }); + + expect(store.getState().editor.project.identifier).toBe("project-456"); + + const url = new URL(screen.getByTitle("Scratch").getAttribute("src")); + expect(url.searchParams.get("project_id")).toBe("project-123"); + }); }); diff --git a/src/components/ProjectBar/ScratchProjectBar.jsx b/src/components/ProjectBar/ScratchProjectBar.jsx index 30f9ec93d..d52059224 100644 --- a/src/components/ProjectBar/ScratchProjectBar.jsx +++ b/src/components/ProjectBar/ScratchProjectBar.jsx @@ -11,6 +11,7 @@ import DesignSystemButton from "../DesignSystemButton/DesignSystemButton"; import "../../assets/stylesheets/ProjectBar.scss"; import { useScratchSaveState } from "../../hooks/useScratchSaveState"; +import { isOwner } from "../../utils/projectHelpers"; const ScratchProjectBar = ({ nameEditable = true }) => { const { t } = useTranslation(); @@ -18,10 +19,20 @@ const ScratchProjectBar = ({ nameEditable = true }) => { const user = useSelector((state) => state.auth.user); const loading = useSelector((state) => state.editor.loading); const readOnly = useSelector((state) => state.editor.readOnly); + const project = useSelector((state) => state.editor.project); + const scratchIframeProjectIdentifier = useSelector( + (state) => state.editor.scratchIframeProjectIdentifier, + ); const showScratchSaveButton = Boolean(user && !readOnly); const enableScratchSaveState = Boolean( loading === "success" && showScratchSaveButton, ); + const shouldRemixOnSave = Boolean( + user && + isOwner(user, project) === false && + project.identifier && + project.identifier === scratchIframeProjectIdentifier, + ); const { isScratchSaving, saveScratchProject, scratchSaveLabelKey } = useScratchSaveState({ enabled: enableScratchSaveState, @@ -58,7 +69,7 @@ const ScratchProjectBar = ({ nameEditable = true }) => {
saveScratchProject({ shouldRemixOnSave })} text={scratchSaveLabel} textAlways icon={} diff --git a/src/components/ProjectBar/ScratchProjectBar.test.js b/src/components/ProjectBar/ScratchProjectBar.test.js index 0fbf0f068..9bbc58ff8 100644 --- a/src/components/ProjectBar/ScratchProjectBar.test.js +++ b/src/components/ProjectBar/ScratchProjectBar.test.js @@ -35,10 +35,12 @@ const user = { const renderScratchProjectBar = (state) => { const middlewares = []; const mockStore = configureStore(middlewares); + const project = state.editor?.project || {}; const store = mockStore({ editor: { loading: "success", - project: {}, + project, + scratchIframeProjectIdentifier: project.identifier || null, ...state.editor, }, auth: { @@ -102,6 +104,48 @@ describe("When project is Scratch", () => { type: "scratch-gui-save", }); }); + + test("clicking Save remixes a non-owner Scratch project on the first save", () => { + renderScratchProjectBar({ + editor: { + project: { + ...scratchProject, + user_id: "teacher-id", + }, + }, + auth: { + user, + }, + }); + + fireEvent.click(screen.getAllByRole("button", { name: "header.save" })[1]); + + expect(postMessageToScratchIframe).toHaveBeenCalledWith({ + type: "scratch-gui-remix", + }); + }); + + test("clicking Save sends scratch-gui-save after the project identifier updates", () => { + renderScratchProjectBar({ + editor: { + project: { + ...scratchProject, + identifier: "student-remix", + user_id: "teacher-id", + }, + scratchIframeProjectIdentifier: "teacher-project", + }, + auth: { + user, + }, + }); + + fireEvent.click(screen.getAllByRole("button", { name: "header.save" })[1]); + + expect(postMessageToScratchIframe).toHaveBeenCalledWith({ + type: "scratch-gui-save", + }); + }); }); describe("Additional Scratch manual save states", () => { @@ -139,6 +183,26 @@ describe("Additional Scratch manual save states", () => { ).toBeInTheDocument(); }); + test("shows the saving state during a Scratch remix", () => { + renderScratchProjectBar({ + editor: { + project: { + ...scratchProject, + user_id: "teacher-id", + }, + }, + auth: { + user, + }, + }); + + dispatchScratchMessage("scratch-gui-remixing-started"); + + expect( + screen.getByRole("button", { name: "saveStatus.saving" }), + ).toBeDisabled(); + }); + test("does not show save for logged-out Scratch users", () => { renderScratchProjectBar({ editor: { diff --git a/src/components/SaveButton/SaveButton.jsx b/src/components/SaveButton/SaveButton.jsx index 3b47ecd5c..b0ed18773 100644 --- a/src/components/SaveButton/SaveButton.jsx +++ b/src/components/SaveButton/SaveButton.jsx @@ -9,6 +9,7 @@ import { isOwner } from "../../utils/projectHelpers"; import DesignSystemButton from "../DesignSystemButton/DesignSystemButton"; import SaveIcon from "../../assets/icons/save.svg"; import { triggerSave } from "../../redux/EditorSlice"; +import { useScratchSaveState } from "../../hooks/useScratchSaveState"; const SaveButton = ({ className, type, fill = false }) => { const dispatch = useDispatch(); @@ -19,6 +20,23 @@ const SaveButton = ({ className, type, fill = false }) => { const webComponent = useSelector((state) => state.editor.webComponent); const user = useSelector((state) => state.auth.user); const project = useSelector((state) => state.editor.project); + const scratchIframeProjectIdentifier = useSelector( + (state) => state.editor.scratchIframeProjectIdentifier, + ); + const isScratchProject = project?.project_type === "code_editor_scratch"; + const enableScratchSaveState = Boolean( + loading === "success" && user && isScratchProject, + ); + const shouldRemixOnSave = Boolean( + enableScratchSaveState && + isOwner(user, project) === false && + project.identifier && + project.identifier === scratchIframeProjectIdentifier, + ); + const { isScratchSaving, saveScratchProject, scratchSaveLabelKey } = + useScratchSaveState({ + enabled: enableScratchSaveState, + }); useEffect(() => { if (!type) { @@ -31,10 +49,26 @@ const SaveButton = ({ className, type, fill = false }) => { window.plausible("Save button"); } document.dispatchEvent(logInEvent); + if (enableScratchSaveState) { + saveScratchProject({ shouldRemixOnSave }); + return; + } dispatch(triggerSave()); - }, [dispatch]); + }, [ + dispatch, + enableScratchSaveState, + saveScratchProject, + shouldRemixOnSave, + ]); const projectOwner = isOwner(user, project); + const buttonText = t( + enableScratchSaveState + ? scratchSaveLabelKey + : user + ? "header.save" + : "header.loginToSave", + ); return ( loading === "success" && @@ -47,11 +81,12 @@ const SaveButton = ({ className, type, fill = false }) => { "btn--tertiary": buttonType === "tertiary", })} onClick={onClickSave} - text={t(user ? "header.save" : "header.loginToSave")} + text={buttonText} textAlways icon={} type={buttonType} fill={fill} + disabled={isScratchSaving} /> ) ); diff --git a/src/components/SaveButton/SaveButton.test.js b/src/components/SaveButton/SaveButton.test.js index 146720e0d..5a4388dc5 100644 --- a/src/components/SaveButton/SaveButton.test.js +++ b/src/components/SaveButton/SaveButton.test.js @@ -4,6 +4,11 @@ import { Provider } from "react-redux"; import configureStore from "redux-mock-store"; import { triggerSave } from "../../redux/EditorSlice"; import SaveButton from "./SaveButton"; +import { postMessageToScratchIframe } from "../../utils/scratchIframe"; + +jest.mock("../../utils/scratchIframe", () => ({ + postMessageToScratchIframe: jest.fn(), +})); const logInHandler = jest.fn(); @@ -15,6 +20,10 @@ describe("When project is loaded", () => { describe("With logged in user", () => { let store; + beforeEach(() => { + jest.clearAllMocks(); + }); + describe("who doesn't own the project", () => { beforeEach(() => { const middlewares = []; @@ -65,6 +74,81 @@ describe("When project is loaded", () => { fireEvent.click(saveButton); expect(logInHandler).toHaveBeenCalled(); }); + + test("clicking save remixes the first save of a Scratch project", () => { + const middlewares = []; + const mockStore = configureStore(middlewares); + const scratchStore = mockStore({ + editor: { + loading: "success", + webComponent: true, + scratchIframeProjectIdentifier: "teacher-project", + project: { + identifier: "teacher-project", + user_id: "teacher-id", + project_type: "code_editor_scratch", + }, + }, + auth: { + user: { + profile: { + user: "student-id", + }, + }, + }, + }); + + render( + + + , + ); + + fireEvent.click(screen.getAllByText("header.save")[1]); + + expect(postMessageToScratchIframe).toHaveBeenCalledWith({ + type: "scratch-gui-remix", + }); + expect(scratchStore.getActions()).toEqual([]); + }); + + test("clicking save uses scratch-gui-save after the project identifier updates", () => { + const middlewares = []; + const mockStore = configureStore(middlewares); + const scratchStore = mockStore({ + editor: { + loading: "success", + webComponent: true, + scratchIframeProjectIdentifier: "teacher-project", + project: { + identifier: "student-remix", + user_id: "teacher-id", + project_type: "code_editor_scratch", + }, + }, + auth: { + user: { + profile: { + user: "student-id", + }, + }, + }, + }); + + render( + + + , + ); + + fireEvent.click(screen.getAllByText("header.save")[1]); + + expect(postMessageToScratchIframe).toHaveBeenCalledWith({ + type: "scratch-gui-save", + }); + expect(scratchStore.getActions()).toEqual([]); + }); + }); describe("who does own the project", () => { @@ -112,6 +196,7 @@ describe("When project is loaded", () => { let store; beforeEach(() => { + jest.clearAllMocks(); const middlewares = []; const mockStore = configureStore(middlewares); const initialState = { @@ -154,6 +239,7 @@ describe("When project is loaded", () => { let store; beforeEach(() => { + jest.clearAllMocks(); const middlewares = []; const mockStore = configureStore(middlewares); const initialState = { @@ -181,6 +267,7 @@ describe("When project is loaded", () => { let store; beforeEach(() => { + jest.clearAllMocks(); const middlewares = []; const mockStore = configureStore(middlewares); const initialState = { diff --git a/src/components/WebComponentProject/WebComponentProject.integration.test.js b/src/components/WebComponentProject/WebComponentProject.integration.test.js index d90a00993..89abb59b1 100644 --- a/src/components/WebComponentProject/WebComponentProject.integration.test.js +++ b/src/components/WebComponentProject/WebComponentProject.integration.test.js @@ -6,7 +6,10 @@ import WebComponentProject from "./WebComponentProject"; import EditorReducer from "../../redux/EditorSlice"; import WebComponentAuthSlice from "../../redux/WebComponentAuthSlice"; import InstructionsSlice from "../../redux/InstructionsSlice"; -import { setProject } from "../../redux/EditorSlice"; +import { + applyScratchProjectIdentifierUpdate, + setProject, +} from "../../redux/EditorSlice"; import { projectIdentifierChangedEvent } from "../../events/WebComponentCustomEvents"; const projectIdentifierChangedHandler = jest.fn(); @@ -22,6 +25,7 @@ let store; describe("WebComponentProject", () => { beforeEach(() => { + projectIdentifierChangedHandler.mockClear(); const rootReducer = combineReducers({ editor: EditorReducer, auth: WebComponentAuthSlice, @@ -83,6 +87,34 @@ describe("WebComponentProject", () => { ); }); }); + + describe("when a Scratch remix updates the project identifier", () => { + beforeEach(() => { + act(() => { + store.dispatch( + setProject({ + identifier: "teacher-project", + project_type: "code_editor_scratch", + components: [], + }), + ); + }); + projectIdentifierChangedHandler.mockClear(); + act(() => { + store.dispatch( + applyScratchProjectIdentifierUpdate({ + projectIdentifier: "student-remix", + }), + ); + }); + }); + + test("triggers projectIdentifierChanged event with the remixed identifier", () => { + expect(projectIdentifierChangedHandler).toHaveBeenCalledWith( + projectIdentifierChangedEvent("student-remix"), + ); + }); + }); }); afterAll(() => { diff --git a/src/redux/EditorSlice.js b/src/redux/EditorSlice.js index c4c1a3190..c3ac922cd 100644 --- a/src/redux/EditorSlice.js +++ b/src/redux/EditorSlice.js @@ -123,6 +123,7 @@ export const editorInitialState = { errorDetails: {}, runnerBeingLoaded: null | "pyodide" | "skulpt", initialComponents: [], + scratchIframeProjectIdentifier: null, }; export const EditorSlice = createSlice({ @@ -207,6 +208,10 @@ export const EditorSlice = createSlice({ extension: c.extension, content: c.content, })); + state.scratchIframeProjectIdentifier = + action.payload.project_type === "code_editor_scratch" + ? action.payload.identifier || null + : null; if (!state.project.image_list) { state.project.image_list = []; } @@ -220,6 +225,13 @@ export const EditorSlice = createSlice({ } state.justLoaded = true; }, + applyScratchProjectIdentifierUpdate: (state, action) => { + if (state.project?.project_type !== "code_editor_scratch") { + return; + } + + state.project.identifier = action.payload.projectIdentifier; + }, setProjectInstructions: (state, action) => { state.project.instructions = action.payload; }, @@ -456,6 +468,7 @@ export const { setHasShownSavePrompt, setWebComponent, setProject, + applyScratchProjectIdentifierUpdate, setProjectInstructions, setReadOnly, setInstructionsEditable, diff --git a/src/redux/EditorSlice.test.js b/src/redux/EditorSlice.test.js index f2b890ccc..4320ac7b9 100644 --- a/src/redux/EditorSlice.test.js +++ b/src/redux/EditorSlice.test.js @@ -15,6 +15,7 @@ import reducer, { updateProjectComponent, setCascadeUpdate, setProject, + applyScratchProjectIdentifierUpdate, } from "./EditorSlice"; const mockCreateRemix = jest.fn(); @@ -862,4 +863,38 @@ describe("initialComponents snapshot", () => { }, ]); }); + + test("setProject stores the original Scratch iframe identifier", () => { + const state = reducer( + undefined, + setProject({ + identifier: "scratch-project", + project_type: "code_editor_scratch", + components: [], + }), + ); + + expect(state.scratchIframeProjectIdentifier).toBe("scratch-project"); + }); + + test("applyScratchProjectIdentifierUpdate updates the current Scratch project identifier only", () => { + const initialState = reducer( + undefined, + setProject({ + identifier: "scratch-project", + project_type: "code_editor_scratch", + components: [], + }), + ); + + const updatedState = reducer( + initialState, + applyScratchProjectIdentifierUpdate({ + projectIdentifier: "student-remix", + }), + ); + + expect(updatedState.project.identifier).toBe("student-remix"); + expect(updatedState.scratchIframeProjectIdentifier).toBe("scratch-project"); + }); }); diff --git a/src/redux/reducers/loadProjectReducers.js b/src/redux/reducers/loadProjectReducers.js index f53613991..eb969db3a 100644 --- a/src/redux/reducers/loadProjectReducers.js +++ b/src/redux/reducers/loadProjectReducers.js @@ -4,6 +4,7 @@ const loadProjectPending = (state, action) => { state.modals = {}; state.currentLoadingRequestId = action.meta.requestId; state.lastSavedTime = null; + state.scratchIframeProjectIdentifier = null; }; const loadProjectFulfilled = (state, action) => { @@ -23,6 +24,10 @@ const loadProjectFulfilled = (state, action) => { state.justLoaded = true; state.saving = "idle"; state.currentLoadingRequestId = undefined; + state.scratchIframeProjectIdentifier = + action.payload.project.project_type === "code_editor_scratch" + ? action.payload.project.identifier || null + : null; state.openFiles = [[]]; const firstPanelIndex = 0; if (state.project.project_type === "html") { diff --git a/src/redux/reducers/loadProjectReducers.test.js b/src/redux/reducers/loadProjectReducers.test.js index 128ebe77b..635005705 100644 --- a/src/redux/reducers/loadProjectReducers.test.js +++ b/src/redux/reducers/loadProjectReducers.test.js @@ -65,6 +65,7 @@ const requestingAProject = function (project, projectFile) { project: project, currentLoadingRequestId: undefined, initialComponents: project.components, + scratchIframeProjectIdentifier: null, }; expect(reducer(initialState, loadFulfilledAction)).toEqual(expectedState); }); From 908264be0b756fa8c5d7357c3f2839764734fc2b Mon Sep 17 00:00:00 2001 From: abcampo-iry <261805581+abcampo-iry@users.noreply.github.com> Date: Fri, 20 Mar 2026 12:52:16 +0100 Subject: [PATCH 3/6] Extract Scratch save helpers Move iframe-specific message handling into shared Scratch helpers and centralize the Scratch save button wiring in a hook so the first-save remix flow reads closer to the regular project save flow. --- .../Editor/Project/ScratchContainer.jsx | 25 +++--- .../ProjectBar/ScratchProjectBar.jsx | 28 ++---- .../ProjectBar/ScratchProjectBar.test.js | 61 +------------ src/components/SaveButton/SaveButton.jsx | 41 +++------ src/components/SaveButton/SaveButton.test.js | 41 +-------- src/hooks/useScratchSave.js | 40 +++++++++ src/hooks/useScratchSaveState.js | 1 - src/utils/scratchIframe.js | 32 ++++++- src/utils/scratchIframe.test.js | 90 +++++++++++++++++-- 9 files changed, 191 insertions(+), 168 deletions(-) create mode 100644 src/hooks/useScratchSave.js diff --git a/src/components/Editor/Project/ScratchContainer.jsx b/src/components/Editor/Project/ScratchContainer.jsx index 716c4c559..ef39af97e 100644 --- a/src/components/Editor/Project/ScratchContainer.jsx +++ b/src/components/Editor/Project/ScratchContainer.jsx @@ -1,6 +1,7 @@ import React, { useEffect } from "react"; import { useDispatch, useSelector } from "react-redux"; import { applyScratchProjectIdentifierUpdate } from "../../../redux/EditorSlice"; +import { subscribeToScratchProjectIdentifierUpdates } from "../../../utils/scratchIframe"; export default function ScratchContainer() { const dispatch = useDispatch(); @@ -17,21 +18,15 @@ export default function ScratchContainer() { scratchIframeProjectIdentifier || projectIdentifier; useEffect(() => { - const allowedOrigin = process.env.ASSETS_URL || window.location.origin; - const handleScratchMessage = ({ origin, data }) => { - if (origin !== allowedOrigin) return; - if (data?.type !== "scratch-gui-project-id-updated") return; - if (!data.projectId) return; - - dispatch( - applyScratchProjectIdentifierUpdate({ - projectIdentifier: data.projectId, - }), - ); - }; - - window.addEventListener("message", handleScratchMessage); - return () => window.removeEventListener("message", handleScratchMessage); + return subscribeToScratchProjectIdentifierUpdates( + (nextProjectIdentifier) => { + dispatch( + applyScratchProjectIdentifierUpdate({ + projectIdentifier: nextProjectIdentifier, + }), + ); + }, + ); }, [dispatch]); const queryParams = new URLSearchParams(); diff --git a/src/components/ProjectBar/ScratchProjectBar.jsx b/src/components/ProjectBar/ScratchProjectBar.jsx index d52059224..157b40d23 100644 --- a/src/components/ProjectBar/ScratchProjectBar.jsx +++ b/src/components/ProjectBar/ScratchProjectBar.jsx @@ -10,8 +10,7 @@ import UploadButton from "../UploadButton/UploadButton"; import DesignSystemButton from "../DesignSystemButton/DesignSystemButton"; import "../../assets/stylesheets/ProjectBar.scss"; -import { useScratchSaveState } from "../../hooks/useScratchSaveState"; -import { isOwner } from "../../utils/projectHelpers"; +import { useScratchSave } from "../../hooks/useScratchSave"; const ScratchProjectBar = ({ nameEditable = true }) => { const { t } = useTranslation(); @@ -19,24 +18,15 @@ const ScratchProjectBar = ({ nameEditable = true }) => { const user = useSelector((state) => state.auth.user); const loading = useSelector((state) => state.editor.loading); const readOnly = useSelector((state) => state.editor.readOnly); - const project = useSelector((state) => state.editor.project); - const scratchIframeProjectIdentifier = useSelector( - (state) => state.editor.scratchIframeProjectIdentifier, - ); const showScratchSaveButton = Boolean(user && !readOnly); - const enableScratchSaveState = Boolean( - loading === "success" && showScratchSaveButton, - ); - const shouldRemixOnSave = Boolean( - user && - isOwner(user, project) === false && - project.identifier && - project.identifier === scratchIframeProjectIdentifier, - ); - const { isScratchSaving, saveScratchProject, scratchSaveLabelKey } = - useScratchSaveState({ - enabled: enableScratchSaveState, - }); + const { + isScratchSaving, + saveScratchProject, + scratchSaveLabelKey, + shouldRemixOnSave, + } = useScratchSave({ + enabled: showScratchSaveButton, + }); const scratchSaveLabel = t(scratchSaveLabelKey); if (loading !== "success") { diff --git a/src/components/ProjectBar/ScratchProjectBar.test.js b/src/components/ProjectBar/ScratchProjectBar.test.js index 9bbc58ff8..0cfe6c8ae 100644 --- a/src/components/ProjectBar/ScratchProjectBar.test.js +++ b/src/components/ProjectBar/ScratchProjectBar.test.js @@ -9,6 +9,9 @@ import { postMessageToScratchIframe } from "../../utils/scratchIframe"; jest.mock("axios"); jest.mock("../../utils/scratchIframe", () => ({ postMessageToScratchIframe: jest.fn(), + shouldRemixScratchProjectOnSave: jest.requireActual( + "../../utils/scratchIframe", + ).shouldRemixScratchProjectOnSave, })); jest.mock("react-router-dom", () => ({ @@ -124,28 +127,6 @@ describe("When project is Scratch", () => { type: "scratch-gui-remix", }); }); - - test("clicking Save sends scratch-gui-save after the project identifier updates", () => { - renderScratchProjectBar({ - editor: { - project: { - ...scratchProject, - identifier: "student-remix", - user_id: "teacher-id", - }, - scratchIframeProjectIdentifier: "teacher-project", - }, - auth: { - user, - }, - }); - - fireEvent.click(screen.getAllByRole("button", { name: "header.save" })[1]); - - expect(postMessageToScratchIframe).toHaveBeenCalledWith({ - type: "scratch-gui-save", - }); - }); }); describe("Additional Scratch manual save states", () => { @@ -213,40 +194,4 @@ describe("Additional Scratch manual save states", () => { expect(screen.queryByText("header.save")).not.toBeInTheDocument(); expect(screen.queryByText("header.loginToSave")).not.toBeInTheDocument(); }); - - test("shows save for logged-in non-owners", () => { - renderScratchProjectBar({ - editor: { - project: { - ...scratchProject, - user_id: "teacher-id", - }, - }, - auth: { - user, - }, - }); - - expect( - screen.getByRole("button", { name: "header.save" }), - ).toBeInTheDocument(); - }); - - test("shows save for logged-in users without a Scratch project identifier", () => { - renderScratchProjectBar({ - editor: { - project: { - ...scratchProject, - identifier: null, - }, - }, - auth: { - user, - }, - }); - - expect( - screen.getByRole("button", { name: "header.save" }), - ).toBeInTheDocument(); - }); }); diff --git a/src/components/SaveButton/SaveButton.jsx b/src/components/SaveButton/SaveButton.jsx index b0ed18773..8518cc004 100644 --- a/src/components/SaveButton/SaveButton.jsx +++ b/src/components/SaveButton/SaveButton.jsx @@ -4,39 +4,28 @@ import { useTranslation } from "react-i18next"; import classNames from "classnames"; import { logInEvent } from "../../events/WebComponentCustomEvents"; -import { isOwner } from "../../utils/projectHelpers"; import DesignSystemButton from "../DesignSystemButton/DesignSystemButton"; import SaveIcon from "../../assets/icons/save.svg"; import { triggerSave } from "../../redux/EditorSlice"; -import { useScratchSaveState } from "../../hooks/useScratchSaveState"; +import { useScratchSave } from "../../hooks/useScratchSave"; const SaveButton = ({ className, type, fill = false }) => { const dispatch = useDispatch(); const { t } = useTranslation(); const [buttonType, setButtonType] = useState(type); - const loading = useSelector((state) => state.editor.loading); const webComponent = useSelector((state) => state.editor.webComponent); - const user = useSelector((state) => state.auth.user); - const project = useSelector((state) => state.editor.project); - const scratchIframeProjectIdentifier = useSelector( - (state) => state.editor.scratchIframeProjectIdentifier, - ); - const isScratchProject = project?.project_type === "code_editor_scratch"; - const enableScratchSaveState = Boolean( - loading === "success" && user && isScratchProject, - ); - const shouldRemixOnSave = Boolean( - enableScratchSaveState && - isOwner(user, project) === false && - project.identifier && - project.identifier === scratchIframeProjectIdentifier, - ); - const { isScratchSaving, saveScratchProject, scratchSaveLabelKey } = - useScratchSaveState({ - enabled: enableScratchSaveState, - }); + const { + enableScratchSaveState, + isScratchSaving, + loading, + projectOwner, + saveScratchProject, + scratchSaveLabelKey, + shouldRemixOnSave, + user, + } = useScratchSave(); useEffect(() => { if (!type) { @@ -54,14 +43,8 @@ const SaveButton = ({ className, type, fill = false }) => { return; } dispatch(triggerSave()); - }, [ - dispatch, - enableScratchSaveState, - saveScratchProject, - shouldRemixOnSave, - ]); + }, [dispatch, enableScratchSaveState, saveScratchProject, shouldRemixOnSave]); - const projectOwner = isOwner(user, project); const buttonText = t( enableScratchSaveState ? scratchSaveLabelKey diff --git a/src/components/SaveButton/SaveButton.test.js b/src/components/SaveButton/SaveButton.test.js index 5a4388dc5..19d7f676b 100644 --- a/src/components/SaveButton/SaveButton.test.js +++ b/src/components/SaveButton/SaveButton.test.js @@ -8,6 +8,9 @@ import { postMessageToScratchIframe } from "../../utils/scratchIframe"; jest.mock("../../utils/scratchIframe", () => ({ postMessageToScratchIframe: jest.fn(), + shouldRemixScratchProjectOnSave: jest.requireActual( + "../../utils/scratchIframe", + ).shouldRemixScratchProjectOnSave, })); const logInHandler = jest.fn(); @@ -111,44 +114,6 @@ describe("When project is loaded", () => { }); expect(scratchStore.getActions()).toEqual([]); }); - - test("clicking save uses scratch-gui-save after the project identifier updates", () => { - const middlewares = []; - const mockStore = configureStore(middlewares); - const scratchStore = mockStore({ - editor: { - loading: "success", - webComponent: true, - scratchIframeProjectIdentifier: "teacher-project", - project: { - identifier: "student-remix", - user_id: "teacher-id", - project_type: "code_editor_scratch", - }, - }, - auth: { - user: { - profile: { - user: "student-id", - }, - }, - }, - }); - - render( - - - , - ); - - fireEvent.click(screen.getAllByText("header.save")[1]); - - expect(postMessageToScratchIframe).toHaveBeenCalledWith({ - type: "scratch-gui-save", - }); - expect(scratchStore.getActions()).toEqual([]); - }); - }); describe("who does own the project", () => { diff --git a/src/hooks/useScratchSave.js b/src/hooks/useScratchSave.js new file mode 100644 index 000000000..6f6f965f5 --- /dev/null +++ b/src/hooks/useScratchSave.js @@ -0,0 +1,40 @@ +import { useSelector } from "react-redux"; +import { isOwner } from "../utils/projectHelpers"; +import { shouldRemixScratchProjectOnSave } from "../utils/scratchIframe"; +import { useScratchSaveState } from "./useScratchSaveState"; + +export const useScratchSave = ({ enabled = true } = {}) => { + const loading = useSelector((state) => state.editor.loading); + const user = useSelector((state) => state.auth.user); + const project = useSelector((state) => state.editor.project); + const scratchIframeProjectIdentifier = useSelector( + (state) => state.editor.scratchIframeProjectIdentifier, + ); + + const projectOwner = isOwner(user, project); + const isScratchProject = project?.project_type === "code_editor_scratch"; + const enableScratchSaveState = Boolean( + enabled && loading === "success" && user && isScratchProject, + ); + const shouldRemixOnSave = shouldRemixScratchProjectOnSave({ + user, + identifier: project?.identifier, + projectOwner, + scratchIframeProjectIdentifier, + }); + const { isScratchSaving, saveScratchProject, scratchSaveLabelKey } = + useScratchSaveState({ + enabled: enableScratchSaveState, + }); + + return { + enableScratchSaveState, + isScratchSaving, + loading, + projectOwner, + saveScratchProject, + scratchSaveLabelKey, + shouldRemixOnSave, + user, + }; +}; diff --git a/src/hooks/useScratchSaveState.js b/src/hooks/useScratchSaveState.js index c7c0c389b..392001108 100644 --- a/src/hooks/useScratchSaveState.js +++ b/src/hooks/useScratchSaveState.js @@ -31,7 +31,6 @@ export const useScratchSaveState = ({ enabled = false } = {}) => { } const allowedOrigin = process.env.ASSETS_URL || window.location.origin; - const handleScratchMessage = (event) => { if (event.origin !== allowedOrigin) { return; diff --git a/src/utils/scratchIframe.js b/src/utils/scratchIframe.js index 63527c7ea..144e5dec3 100644 --- a/src/utils/scratchIframe.js +++ b/src/utils/scratchIframe.js @@ -5,5 +5,35 @@ export const getScratchIframeContentWindow = () => { }; export const postMessageToScratchIframe = (message) => { - getScratchIframeContentWindow().postMessage(message, process.env.ASSETS_URL); + const allowedOrigin = process.env.ASSETS_URL || window.location.origin; + getScratchIframeContentWindow().postMessage(message, allowedOrigin); +}; + +export const shouldRemixScratchProjectOnSave = ({ + user, + identifier, + projectOwner, + scratchIframeProjectIdentifier, +}) => { + return Boolean( + !projectOwner && + user && + identifier && + identifier === scratchIframeProjectIdentifier, + ); +}; + +export const subscribeToScratchProjectIdentifierUpdates = (handler) => { + const allowedOrigin = process.env.ASSETS_URL || window.location.origin; + + const handleScratchMessage = ({ origin, data }) => { + if (origin !== allowedOrigin) return; + if (data?.type !== "scratch-gui-project-id-updated") return; + if (!data.projectId) return; + + handler(data.projectId); + }; + + window.addEventListener("message", handleScratchMessage); + return () => window.removeEventListener("message", handleScratchMessage); }; diff --git a/src/utils/scratchIframe.test.js b/src/utils/scratchIframe.test.js index 4c44a54a7..495950b17 100644 --- a/src/utils/scratchIframe.test.js +++ b/src/utils/scratchIframe.test.js @@ -1,6 +1,8 @@ import { getScratchIframeContentWindow, postMessageToScratchIframe, + shouldRemixScratchProjectOnSave, + subscribeToScratchProjectIdentifierUpdates, } from "./scratchIframe"; describe("scratchIframe", () => { @@ -33,13 +35,6 @@ describe("scratchIframe", () => { expect(document.querySelector).toHaveBeenCalledWith("editor-wc"); expect(result).toBe(mockContentWindow); }); - - it("queries iframe by Scratch title", () => { - getScratchIframeContentWindow(); - expect(mockShadowQuerySelector).toHaveBeenCalledWith( - "iframe[title='Scratch']", - ); - }); }); describe("postMessageToScratchIframe", () => { @@ -66,4 +61,85 @@ describe("scratchIframe", () => { ); }); }); + + describe("subscribeToScratchProjectIdentifierUpdates", () => { + const originalAssetsUrl = process.env.ASSETS_URL; + + beforeEach(() => { + process.env.ASSETS_URL = "https://assets.example.com"; + }); + + afterEach(() => { + process.env.ASSETS_URL = originalAssetsUrl; + }); + + it("calls the handler with the updated project id", () => { + const handler = jest.fn(); + const unsubscribe = subscribeToScratchProjectIdentifierUpdates(handler); + + window.dispatchEvent( + new MessageEvent("message", { + origin: "https://assets.example.com", + data: { + type: "scratch-gui-project-id-updated", + projectId: "project-456", + }, + }), + ); + + expect(handler).toHaveBeenCalledWith("project-456"); + unsubscribe(); + }); + + it("ignores unrelated messages", () => { + const handler = jest.fn(); + const unsubscribe = subscribeToScratchProjectIdentifierUpdates(handler); + + window.dispatchEvent( + new MessageEvent("message", { + origin: "https://other.example.com", + data: { + type: "scratch-gui-project-id-updated", + projectId: "project-456", + }, + }), + ); + + window.dispatchEvent( + new MessageEvent("message", { + origin: "https://assets.example.com", + data: { + type: "scratch-gui-saving-succeeded", + }, + }), + ); + + expect(handler).not.toHaveBeenCalled(); + unsubscribe(); + }); + }); + + describe("shouldRemixScratchProjectOnSave", () => { + it("returns true for the first save of an educator-owned project", () => { + expect( + shouldRemixScratchProjectOnSave({ + user: { profile: { user: "student-id" } }, + identifier: "teacher-project", + projectOwner: false, + scratchIframeProjectIdentifier: "teacher-project", + }), + ).toBe(true); + }); + + it("returns false after the parent project id has been updated", () => { + expect( + shouldRemixScratchProjectOnSave({ + user: { profile: { user: "student-id" } }, + identifier: "student-remix", + projectOwner: false, + scratchIframeProjectIdentifier: "teacher-project", + }), + ).toBe(false); + }); + }); }); From c0ab97185ebc819e22efe7b4bbca1db4a6a81946 Mon Sep 17 00:00:00 2001 From: abcampo-iry <261805581+abcampo-iry@users.noreply.github.com> Date: Tue, 24 Mar 2026 13:30:23 +0100 Subject: [PATCH 4/6] Keep Scratch saves on the existing iframe session --- .../Editor/Project/ScratchContainer.test.js | 5 +- src/containers/WebComponentLoader.jsx | 9 ++- src/containers/WebComponentLoader.test.js | 61 ++++++++++++++++++- src/redux/EditorSlice.test.js | 2 +- 4 files changed, 72 insertions(+), 5 deletions(-) diff --git a/src/components/Editor/Project/ScratchContainer.test.js b/src/components/Editor/Project/ScratchContainer.test.js index a51c596d4..18045b0e4 100644 --- a/src/components/Editor/Project/ScratchContainer.test.js +++ b/src/components/Editor/Project/ScratchContainer.test.js @@ -49,7 +49,7 @@ describe("ScratchContainer", () => { expect(url.searchParams.get("api_url")).toBe("https://api.example.com/v1"); }); - test("updates the parent project identifier without changing the iframe project_id", () => { + test("updates the parent project identifier without reloading the iframe project_id", () => { const store = configureStore({ reducer: { editor: EditorReducer, @@ -85,6 +85,9 @@ describe("ScratchContainer", () => { }); expect(store.getState().editor.project.identifier).toBe("project-456"); + expect(store.getState().editor.scratchIframeProjectIdentifier).toBe( + "project-123", + ); const url = new URL(screen.getByTitle("Scratch").getAttribute("src")); expect(url.searchParams.get("project_id")).toBe("project-123"); diff --git a/src/containers/WebComponentLoader.jsx b/src/containers/WebComponentLoader.jsx index 80d09dbcd..e626aec58 100644 --- a/src/containers/WebComponentLoader.jsx +++ b/src/containers/WebComponentLoader.jsx @@ -120,10 +120,15 @@ const WebComponentLoader = (props) => { }, [theme, setCookie, dispatch]); useEffect(() => { - if (loading === "idle" && project.identifier) { + if ( + loading === "idle" && + project.project_type !== "code_editor_scratch" && + project.identifier && + project.identifier !== projectIdentifier + ) { setProjectIdentifier(project.identifier); } - }, [loading, project]); + }, [loading, project.project_type, project.identifier, projectIdentifier]); useEffect(() => { if (loading === "failed" && !remixLoadFailed) { diff --git a/src/containers/WebComponentLoader.test.js b/src/containers/WebComponentLoader.test.js index e6489ed8d..04317dc39 100644 --- a/src/containers/WebComponentLoader.test.js +++ b/src/containers/WebComponentLoader.test.js @@ -1,14 +1,18 @@ -import { render, act } from "@testing-library/react"; +import { render, act, waitFor } from "@testing-library/react"; import React from "react"; import { Provider } from "react-redux"; import configureStore from "redux-mock-store"; +import { configureStore as configureRealStore } from "@reduxjs/toolkit"; import WebComponentLoader from "./WebComponentLoader"; import { + applyScratchProjectIdentifierUpdate, disableTheming, + editorInitialState, setReadOnly, setSenseHatAlwaysEnabled, setReactAppApiEndpoint, setScratchApiEndpoint, + default as EditorReducer, } from "../redux/EditorSlice"; import { setInstructions } from "../redux/InstructionsSlice"; import { setUser } from "../redux/WebComponentAuthSlice"; @@ -717,3 +721,58 @@ describe("When user is in state", () => { }); }); }); + +describe("when a Scratch remix updates the project identifier", () => { + test("keeps the existing project load state instead of reloading the iframe project", async () => { + const store = configureRealStore({ + reducer: { + editor: EditorReducer, + instructions: (state = {}) => state, + auth: (state = { user }) => state, + }, + preloadedState: { + editor: { + ...editorInitialState, + loading: "success", + project: { + identifier: "teacher-project", + project_type: "code_editor_scratch", + components: [], + }, + }, + }, + auth: { user }, + }); + + render( + + + + + , + ); + + act(() => { + store.dispatch( + applyScratchProjectIdentifierUpdate({ + projectIdentifier: "student-remix", + }), + ); + }); + + await waitFor(() => { + expect(useProject).toHaveBeenLastCalledWith({ + assetsIdentifier: undefined, + projectIdentifier: "teacher-project", + code: undefined, + initialProject: null, + accessToken: "my_token", + loadRemix: true, + locale: "en", + loadCache: true, + remixLoadFailed: false, + reactAppApiEndpoint: "http://localhost:3009", + }); + }); + }); +}); diff --git a/src/redux/EditorSlice.test.js b/src/redux/EditorSlice.test.js index 4320ac7b9..5090ce66a 100644 --- a/src/redux/EditorSlice.test.js +++ b/src/redux/EditorSlice.test.js @@ -877,7 +877,7 @@ describe("initialComponents snapshot", () => { expect(state.scratchIframeProjectIdentifier).toBe("scratch-project"); }); - test("applyScratchProjectIdentifierUpdate updates the current Scratch project identifier only", () => { + test("applyScratchProjectIdentifierUpdate updates the parent Scratch project identifier without reloading the iframe", () => { const initialState = reducer( undefined, setProject({ From 9768186f249d507b8303c14dc9fd0a56e8a82064 Mon Sep 17 00:00:00 2001 From: abcampo-iry <261805581+abcampo-iry@users.noreply.github.com> Date: Tue, 24 Mar 2026 13:30:23 +0100 Subject: [PATCH 5/6] Keep Scratch save logic out of SaveButton --- src/components/SaveButton/SaveButton.jsx | 32 +++--------- src/components/SaveButton/SaveButton.test.js | 52 -------------------- src/hooks/useScratchSave.js | 8 +-- 3 files changed, 11 insertions(+), 81 deletions(-) diff --git a/src/components/SaveButton/SaveButton.jsx b/src/components/SaveButton/SaveButton.jsx index 8518cc004..3b47ecd5c 100644 --- a/src/components/SaveButton/SaveButton.jsx +++ b/src/components/SaveButton/SaveButton.jsx @@ -4,28 +4,21 @@ import { useTranslation } from "react-i18next"; import classNames from "classnames"; import { logInEvent } from "../../events/WebComponentCustomEvents"; +import { isOwner } from "../../utils/projectHelpers"; import DesignSystemButton from "../DesignSystemButton/DesignSystemButton"; import SaveIcon from "../../assets/icons/save.svg"; import { triggerSave } from "../../redux/EditorSlice"; -import { useScratchSave } from "../../hooks/useScratchSave"; const SaveButton = ({ className, type, fill = false }) => { const dispatch = useDispatch(); const { t } = useTranslation(); const [buttonType, setButtonType] = useState(type); + const loading = useSelector((state) => state.editor.loading); const webComponent = useSelector((state) => state.editor.webComponent); - const { - enableScratchSaveState, - isScratchSaving, - loading, - projectOwner, - saveScratchProject, - scratchSaveLabelKey, - shouldRemixOnSave, - user, - } = useScratchSave(); + const user = useSelector((state) => state.auth.user); + const project = useSelector((state) => state.editor.project); useEffect(() => { if (!type) { @@ -38,20 +31,10 @@ const SaveButton = ({ className, type, fill = false }) => { window.plausible("Save button"); } document.dispatchEvent(logInEvent); - if (enableScratchSaveState) { - saveScratchProject({ shouldRemixOnSave }); - return; - } dispatch(triggerSave()); - }, [dispatch, enableScratchSaveState, saveScratchProject, shouldRemixOnSave]); + }, [dispatch]); - const buttonText = t( - enableScratchSaveState - ? scratchSaveLabelKey - : user - ? "header.save" - : "header.loginToSave", - ); + const projectOwner = isOwner(user, project); return ( loading === "success" && @@ -64,12 +47,11 @@ const SaveButton = ({ className, type, fill = false }) => { "btn--tertiary": buttonType === "tertiary", })} onClick={onClickSave} - text={buttonText} + text={t(user ? "header.save" : "header.loginToSave")} textAlways icon={} type={buttonType} fill={fill} - disabled={isScratchSaving} /> ) ); diff --git a/src/components/SaveButton/SaveButton.test.js b/src/components/SaveButton/SaveButton.test.js index 19d7f676b..146720e0d 100644 --- a/src/components/SaveButton/SaveButton.test.js +++ b/src/components/SaveButton/SaveButton.test.js @@ -4,14 +4,6 @@ import { Provider } from "react-redux"; import configureStore from "redux-mock-store"; import { triggerSave } from "../../redux/EditorSlice"; import SaveButton from "./SaveButton"; -import { postMessageToScratchIframe } from "../../utils/scratchIframe"; - -jest.mock("../../utils/scratchIframe", () => ({ - postMessageToScratchIframe: jest.fn(), - shouldRemixScratchProjectOnSave: jest.requireActual( - "../../utils/scratchIframe", - ).shouldRemixScratchProjectOnSave, -})); const logInHandler = jest.fn(); @@ -23,10 +15,6 @@ describe("When project is loaded", () => { describe("With logged in user", () => { let store; - beforeEach(() => { - jest.clearAllMocks(); - }); - describe("who doesn't own the project", () => { beforeEach(() => { const middlewares = []; @@ -77,43 +65,6 @@ describe("When project is loaded", () => { fireEvent.click(saveButton); expect(logInHandler).toHaveBeenCalled(); }); - - test("clicking save remixes the first save of a Scratch project", () => { - const middlewares = []; - const mockStore = configureStore(middlewares); - const scratchStore = mockStore({ - editor: { - loading: "success", - webComponent: true, - scratchIframeProjectIdentifier: "teacher-project", - project: { - identifier: "teacher-project", - user_id: "teacher-id", - project_type: "code_editor_scratch", - }, - }, - auth: { - user: { - profile: { - user: "student-id", - }, - }, - }, - }); - - render( - - - , - ); - - fireEvent.click(screen.getAllByText("header.save")[1]); - - expect(postMessageToScratchIframe).toHaveBeenCalledWith({ - type: "scratch-gui-remix", - }); - expect(scratchStore.getActions()).toEqual([]); - }); }); describe("who does own the project", () => { @@ -161,7 +112,6 @@ describe("When project is loaded", () => { let store; beforeEach(() => { - jest.clearAllMocks(); const middlewares = []; const mockStore = configureStore(middlewares); const initialState = { @@ -204,7 +154,6 @@ describe("When project is loaded", () => { let store; beforeEach(() => { - jest.clearAllMocks(); const middlewares = []; const mockStore = configureStore(middlewares); const initialState = { @@ -232,7 +181,6 @@ describe("When project is loaded", () => { let store; beforeEach(() => { - jest.clearAllMocks(); const middlewares = []; const mockStore = configureStore(middlewares); const initialState = { diff --git a/src/hooks/useScratchSave.js b/src/hooks/useScratchSave.js index 6f6f965f5..f72687773 100644 --- a/src/hooks/useScratchSave.js +++ b/src/hooks/useScratchSave.js @@ -4,11 +4,11 @@ import { shouldRemixScratchProjectOnSave } from "../utils/scratchIframe"; import { useScratchSaveState } from "./useScratchSaveState"; export const useScratchSave = ({ enabled = true } = {}) => { - const loading = useSelector((state) => state.editor.loading); - const user = useSelector((state) => state.auth.user); - const project = useSelector((state) => state.editor.project); + const loading = useSelector((state) => state.editor?.loading); + const user = useSelector((state) => state.auth?.user); + const project = useSelector((state) => state.editor?.project); const scratchIframeProjectIdentifier = useSelector( - (state) => state.editor.scratchIframeProjectIdentifier, + (state) => state.editor?.scratchIframeProjectIdentifier, ); const projectOwner = isOwner(user, project); From d18282787ef80c4bd718324fa63ee622d65ab277 Mon Sep 17 00:00:00 2001 From: abcampo-iry <261805581+abcampo-iry@users.noreply.github.com> Date: Wed, 25 Mar 2026 12:19:26 +0100 Subject: [PATCH 6/6] cypress test covering --- cypress/e2e/spec-scratch.cy.js | 78 ++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/cypress/e2e/spec-scratch.cy.js b/cypress/e2e/spec-scratch.cy.js index 4a6933c40..2f68841e1 100644 --- a/cypress/e2e/spec-scratch.cy.js +++ b/cypress/e2e/spec-scratch.cy.js @@ -8,6 +8,13 @@ import { } from "../helpers/scratch.js"; const origin = "http://localhost:3011/web-component.html"; +const authKey = "oidc.user:https://auth-v1.raspberrypi.org:editor-api"; +const user = { + access_token: "dummy-access-token", + profile: { + user: "student-id", + }, +}; beforeEach(() => { cy.intercept("*", (req) => { @@ -75,3 +82,74 @@ describe("Scratch", () => { }); }); }); + +describe("Scratch save integration", () => { + beforeEach(() => { + cy.on("window:before:load", (win) => { + win.localStorage.setItem(authKey, JSON.stringify(user)); + }); + + const params = new URLSearchParams(); + params.set("auth_key", authKey); + params.set("load_remix_disabled", "true"); + + cy.visit(`${origin}?${params.toString()}`); + cy.findByText("cool-scratch").click(); + }); + + it("remixes on the first save, keeps the iframe project loaded, and saves after the identifier update", () => { + getEditorShadow() + .find("iframe[title='Scratch']") + .its("0.contentDocument.body") + .should("not.be.empty"); + + getEditorShadow() + .find("iframe[title='Scratch']") + .should(($iframe) => { + const url = new URL($iframe.attr("src")); + expect(url.searchParams.get("project_id")).to.eq("cool-scratch.json"); + }) + .then(($iframe) => { + cy.stub($iframe[0].contentWindow, "postMessage").as( + "scratchPostMessage", + ); + }); + + getEditorShadow().findByRole("button", { name: "Save" }).click(); + + cy.get("@scratchPostMessage") + .its("firstCall.args.0") + .should("deep.include", { type: "scratch-gui-remix" }); + + cy.window().then((win) => { + win.dispatchEvent( + new win.MessageEvent("message", { + origin: win.location.origin, + data: { + type: "scratch-gui-project-id-updated", + projectId: "student-remix", + }, + }), + ); + }); + + cy.get("#project-identifier").should("have.text", "student-remix"); + + getEditorShadow() + .find("iframe[title='Scratch']") + .should(($iframe) => { + const url = new URL($iframe.attr("src")); + expect(url.searchParams.get("project_id")).to.eq("cool-scratch.json"); + }); + + cy.get("@scratchPostMessage").then((postMessage) => { + postMessage.resetHistory(); + }); + + getEditorShadow().findByRole("button", { name: "Save" }).click(); + + cy.get("@scratchPostMessage") + .its("firstCall.args.0") + .should("deep.include", { type: "scratch-gui-save" }); + }); +});