Skip to content

Commit

Permalink
Fix mod component deletion behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
twschiller committed Oct 26, 2024
1 parent 2a1598a commit 175c7f0
Show file tree
Hide file tree
Showing 11 changed files with 233 additions and 68 deletions.
38 changes: 30 additions & 8 deletions src/pageEditor/hooks/useCreateModFromModComponent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@
*/

import useCreateModFromModComponent from "@/pageEditor/hooks/useCreateModFromModComponent";
import { hookAct, renderHook, waitFor } from "@/pageEditor/testHelpers";
import { renderHook, waitFor } from "@/pageEditor/testHelpers";
import { appApiMock } from "@/testUtils/appApiMock";
import { modMetadataFactory } from "@/testUtils/factories/modComponentFactories";
import { menuItemFormStateFactory } from "@/testUtils/factories/pageEditorFactories";
import reportEvent from "@/telemetry/reportEvent";
import { Events } from "@/telemetry/events";
import { API_PATHS } from "@/data/service/urlPaths";
import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice";

const reportEventMock = jest.mocked(reportEvent);

Expand All @@ -42,7 +43,7 @@ describe("useCreateModFromModComponent", () => {
appApiMock.reset();
});

it("saves with no dirty changes", async () => {
it("creates mod with no dirty changes", async () => {
const metadata = modMetadataFactory();
const menuItemFormState = menuItemFormStateFactory({
modMetadata: metadata,
Expand All @@ -54,15 +55,15 @@ describe("useCreateModFromModComponent", () => {

appApiMock.onGet(API_PATHS.BRICKS).reply(200, []);

const { result } = renderHook(() => useCreateModFromModComponent());

await hookAct(async () => {
// Wait for editable packages to be created
const { result, act } = renderHook(() => useCreateModFromModComponent(), {
setupRedux(dispatch) {
dispatch(editorActions.addModComponentFormState(menuItemFormState));
},
});

await hookAct(async () => {
await act(async () => {
await result.current.createModFromComponent(menuItemFormState, metadata, {
keepLocalCopy: false,
keepLocalCopy: true,
});
});

Expand All @@ -75,4 +76,25 @@ describe("useCreateModFromModComponent", () => {
);
});
});

it("errors moving last mod component in a mod", async () => {
const metadata = modMetadataFactory();
const menuItemFormState = menuItemFormStateFactory({
modMetadata: metadata,
});

const { result } = renderHook(() => useCreateModFromModComponent(), {
setupRedux(dispatch) {
dispatch(editorActions.addModComponentFormState(menuItemFormState));
},
});

await expect(
result.current.createModFromComponent(menuItemFormState, metadata, {
keepLocalCopy: false,
}),
).rejects.toThrow("Cannot remove the last starter brick in a mod");

expect(appApiMock.history.post).toHaveLength(0);
});
});
14 changes: 13 additions & 1 deletion src/pageEditor/hooks/useCreateModFromModComponent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@ import reportEvent from "@/telemetry/reportEvent";
import { useCallback } from "react";
import { Events } from "@/telemetry/events";
import { useCreateModDefinitionMutation } from "@/data/service/api";
import { useDispatch } from "react-redux";
import { useDispatch, useSelector } from "react-redux";
import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice";
import { mapModDefinitionUpsertResponseToModDefinition } from "@/pageEditor/utils";
import useDeleteDraftModComponent from "@/pageEditor/hooks/useDeleteDraftModComponent";
import useBuildAndValidateMod from "@/pageEditor/hooks/useBuildAndValidateMod";
import { createPrivateSharing } from "@/utils/registryUtils";
import updateReduxForSavedModDefinition from "@/pageEditor/hooks/updateReduxForSavedModDefinition";
import { type AppDispatch } from "@/pageEditor/store/store";
import { selectGetSiblingDraftModComponents } from "@/pageEditor/store/editor/editorSelectors";

type UseCreateModFromModReturn = {
createModFromComponent: (
Expand All @@ -44,6 +45,9 @@ function useCreateModFromModComponent(): UseCreateModFromModReturn {
const [createModDefinitionOnServer] = useCreateModDefinitionMutation();
const deleteDraftModComponent = useDeleteDraftModComponent();
const { buildAndValidateMod } = useBuildAndValidateMod();
const getSiblingDraftModComponents = useSelector(
selectGetSiblingDraftModComponents,
);

const createModFromComponent = useCallback(
(
Expand All @@ -60,6 +64,13 @@ function useCreateModFromModComponent(): UseCreateModFromModReturn {
return;
}

if (
!keepLocalCopy &&
getSiblingDraftModComponents(modComponentFormState.uuid).length === 1
) {
throw new Error("Cannot remove the last starter brick in a mod");
}

const modId = newModMetadata.id;
const draftModComponents = [modComponentFormState];

Expand Down Expand Up @@ -104,6 +115,7 @@ function useCreateModFromModComponent(): UseCreateModFromModReturn {
createModDefinitionOnServer,
dispatch,
deleteDraftModComponent,
getSiblingDraftModComponents,
],
);

Expand Down
98 changes: 75 additions & 23 deletions src/pageEditor/hooks/useDeleteDraftModComponent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,39 +17,91 @@

import { renderHook } from "@/pageEditor/testHelpers";
import useDeleteDraftModComponent from "./useDeleteDraftModComponent";
import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice";
import {
actions,
actions as editorActions,
} from "@/pageEditor/store/editor/editorSlice";
import { removeDraftModComponents } from "@/contentScript/messenger/api";

import { autoUUIDSequence } from "@/testUtils/factories/stringFactories";
import { formStateFactory } from "@/testUtils/factories/pageEditorFactories";
import { modMetadataFactory } from "@/testUtils/factories/modComponentFactories";

beforeEach(() => {
jest.resetAllMocks();
});

test("useDeleteModComponent", async () => {
const modComponentId = autoUUIDSequence();
describe("useDeleteModComponent", () => {
it("deletes one component from mod", async () => {
const modMetadata = modMetadataFactory();

const {
result: { current: deleteDraftModComponent },
getReduxStore,
} = renderHook(() => useDeleteDraftModComponent(), {
setupRedux(_dispatch, { store }) {
jest.spyOn(store, "dispatch");
},
});
const formState = formStateFactory({
formStateConfig: {
modMetadata,
},
});

const { uuid: modComponentId } = formState;

const {
result: { current: deleteDraftModComponent },
getReduxStore,
} = renderHook(() => useDeleteDraftModComponent(), {
setupRedux(dispatch, { store }) {
jest.spyOn(store, "dispatch");
// Add another so they can be deleted
dispatch(
actions.addModComponentFormState(
formStateFactory({
formStateConfig: {
modMetadata,
},
}),
),
);

dispatch(actions.addModComponentFormState(formState));
},
});

await deleteDraftModComponent({
modComponentId,
});

await deleteDraftModComponent({
modComponentId,
const { dispatch } = getReduxStore();

expect(dispatch).toHaveBeenCalledWith(
editorActions.markModComponentFormStateAsDeleted(modComponentId),
);

expect(removeDraftModComponents).toHaveBeenCalledWith(
expect.any(Object),
modComponentId,
);
});

const { dispatch } = getReduxStore();
it("does not delete last mod component in mod", async () => {
const formState = formStateFactory();
const { uuid: modComponentId } = formState;

const {
result: { current: deleteDraftModComponent },
getReduxStore,
} = renderHook(() => useDeleteDraftModComponent(), {
setupRedux(dispatch, { store }) {
jest.spyOn(store, "dispatch");
dispatch(actions.addModComponentFormState(formState));
},
});

expect(dispatch).toHaveBeenCalledWith(
editorActions.markModComponentFormStateAsDeleted(modComponentId),
);
await deleteDraftModComponent({
modComponentId,
});

expect(removeDraftModComponents).toHaveBeenCalledWith(
expect.any(Object),
modComponentId,
);
const { dispatch } = getReduxStore();

expect(dispatch).not.toHaveBeenCalledWith(
editorActions.markModComponentFormStateAsDeleted(modComponentId),
);

expect(removeDraftModComponents).not.toHaveBeenCalled();
});
});
21 changes: 20 additions & 1 deletion src/pageEditor/hooks/useDeleteDraftModComponent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice"
import { removeDraftModComponents } from "@/contentScript/messenger/api";
import { allFramesInInspectedTab } from "@/pageEditor/context/connection";
import { selectActivatedModComponentsMap } from "@/store/modComponents/modComponentSelectors";
import { selectGetSiblingDraftModComponents } from "@/pageEditor/store/editor/editorSelectors";

type DeleteConfig = {
modComponentId: UUID;
Expand Down Expand Up @@ -63,13 +64,25 @@ function useDeleteDraftModComponent(): (
) => Promise<void> {
const dispatch = useDispatch();
const sessionId = useSelector(selectSessionId);
const getSiblingDraftModComponents = useSelector(
selectGetSiblingDraftModComponents,
);
const activatedModComponentsMap = useSelector(
selectActivatedModComponentsMap,
);
const { showConfirmation } = useModals();

return useCallback(
async ({ modComponentId, shouldShowConfirmation }) => {
// Prevent deletion of the last mod component in a mod. The editorSlice state currently stores some mod
// information on the mod components/form state.
if (getSiblingDraftModComponents(modComponentId).length === 1) {
notify.warning(
"You cannot delete/remove the last starter brick in a mod",
);
return;
}

if (shouldShowConfirmation) {
const confirm = await showConfirmation(
activatedModComponentsMap.get(modComponentId)
Expand Down Expand Up @@ -102,7 +115,13 @@ function useDeleteDraftModComponent(): (
});
}
},
[dispatch, sessionId, showConfirmation, activatedModComponentsMap],
[
dispatch,
sessionId,
showConfirmation,
activatedModComponentsMap,
getSiblingDraftModComponents,
],
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ import React from "react";
import { render } from "@/pageEditor/testHelpers";
import DraftModComponentListItem from "@/pageEditor/modListingPanel/DraftModComponentListItem";
import { actions as editorActions } from "@/pageEditor/store/editor/editorSlice";
import { authActions } from "@/auth/authSlice";
import { formStateFactory } from "@/testUtils/factories/pageEditorFactories";
import { authStateFactory } from "@/testUtils/factories/authFactories";

jest.mock("@/modDefinitions/modDefinitionHooks", () => ({
useAllModDefinitions: jest
Expand Down Expand Up @@ -55,14 +53,14 @@ describe("DraftModComponentListItem", () => {
{
initialValues: formState,
setupRedux(dispatch) {
dispatch(authActions.setAuth(authStateFactory()));
// The addElement also sets the active element
dispatch(
editorActions.addModComponentFormState(formStateFactory()),
);

// Add new element to deactivate the previous one
dispatch(editorActions.addModComponentFormState(formState));

// Remove the active element and stay with one inactive item
dispatch(
editorActions.markModComponentFormStateAsDeleted(formState.uuid),
Expand All @@ -84,7 +82,6 @@ describe("DraftModComponentListItem", () => {
{
initialValues: formState,
setupRedux(dispatch) {
dispatch(authActions.setAuth(authStateFactory()));
// The addElement also sets the active element
dispatch(editorActions.addModComponentFormState(formState));
},
Expand Down
15 changes: 14 additions & 1 deletion src/pageEditor/modListingPanel/ModComponentActionMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ import EllipsisMenu, {
import useDeleteDraftModComponent from "@/pageEditor/hooks/useDeleteDraftModComponent";
import { type ModComponentFormState } from "@/pageEditor/starterBricks/formStateTypes";
import { useDispatch, useSelector } from "react-redux";
import { selectModComponentIsDirty } from "@/pageEditor/store/editor/editorSelectors";
import {
selectGetSiblingDraftModComponents,
selectModComponentIsDirty,
} from "@/pageEditor/store/editor/editorSelectors";
import useClearModComponentChanges from "@/pageEditor/hooks/useClearModComponentChanges";
import { actions } from "@/pageEditor/store/editor/editorSlice";

Expand All @@ -43,6 +46,14 @@ const ModComponentActionMenu: React.FC<{
const deleteDraftModComponent = useDeleteDraftModComponent();
const clearModComponentChanges = useClearModComponentChanges();

// Prevent deletion of the last mod component in a mod. The editorSlice state currently stores some mod
// information on the mod components/form state.
const getSiblingDraftModComponents = useSelector(
selectGetSiblingDraftModComponents,
);
const allowDelete =
getSiblingDraftModComponents(modComponentFormState.uuid).length > 1;

const isDirty = useSelector(
selectModComponentIsDirty(modComponentFormState.uuid),
);
Expand Down Expand Up @@ -79,6 +90,7 @@ const ModComponentActionMenu: React.FC<{
className={styles.moveIcon}
/>
),
disabled: !allowDelete,
async action() {
dispatch(
actions.showMoveCopyToModModal({
Expand All @@ -103,6 +115,7 @@ const ModComponentActionMenu: React.FC<{
{
title: "Delete",
icon: <FontAwesomeIcon icon={faTrash} fixedWidth />,
disabled: !allowDelete,
async action() {
await deleteDraftModComponent({
modComponentId: modComponentFormState.uuid,
Expand Down
21 changes: 16 additions & 5 deletions src/pageEditor/modListingPanel/ModListingPanel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -165,8 +165,7 @@ describe("ModListingPanel", () => {
expect(screen.getByText(`${formState.label} (Copy)`)).toBeInTheDocument();
});

// Regression test for the existing behavior. In the future, we might try to keep the empty mod item displayed
it("delete last activated mod component removes entire mod", async () => {
it("prevents deleting last mod component in a mod", async () => {
const modDefinition = modDefinitionFactory();
const componentName = modDefinition.extensionPoints[0]!.label;

Expand Down Expand Up @@ -196,11 +195,23 @@ describe("ModListingPanel", () => {

await clickEllipsesMenu(componentName);

await clickMenuItemAndConfirm("Delete");
expect(
screen.getByRole("menuitem", {
name: "Delete",
}),
).toHaveAttribute("aria-disabled", "true");

expect(
screen.getByRole("menuitem", {
name: "Move to mod",
}),
).toHaveAttribute("aria-disabled", "true");

expect(
screen.queryByText(modDefinition.metadata.name),
).not.toBeInTheDocument();
screen.getByRole("menuitem", {
name: "Copy to mod",
}),
).not.toHaveAttribute("aria-disabled", "true");
});

it("deletes an activated mod component and undeletes on clear changes", async () => {
Expand Down
Loading

0 comments on commit 175c7f0

Please sign in to comment.