From 0ce19cc44f4a038e02c5b76bae0bde61362d685a Mon Sep 17 00:00:00 2001 From: Matt Potts Date: Tue, 20 Feb 2024 16:39:57 +0000 Subject: [PATCH] Fix: Deleting components does not remove their references 285 (#286) Co-authored-by: Matt Potts --- docs/_config.yml | 2 +- package.json | 2 +- .../CraftingSystemEditor.svelte | 15 ++-- .../CraftingSystemEditor.ts | 20 +++-- .../componentManager/ComponentEditor.svelte | 3 +- .../CraftingComponentEditor.ts | 19 +++- .../essenceManager/EssenceEditor.ts | 39 ++++++-- test/RecipeAPI.test.ts | 89 ++++++++++++++----- test/stubs/api/StubComponentAPI.ts | 7 ++ 9 files changed, 149 insertions(+), 47 deletions(-) diff --git a/docs/_config.yml b/docs/_config.yml index 693821f3..70f21054 100644 --- a/docs/_config.yml +++ b/docs/_config.yml @@ -1,4 +1,4 @@ -title: Fabricate 0.10.20 +title: Fabricate 0.10.21 email: matt@misterpotts.uk description: >- End user documentation for the Foundry Virtual Tabletop (VTT) Module, "Fabricate". diff --git a/package.json b/package.json index c6d72b17..a100e0e7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "fabricate", - "version": "0.10.20", + "version": "0.10.21", "description": "A system-agnostic, flexible crafting module for FoundryVT", "main": "index.js", "type": "module", diff --git a/src/applications/craftingSystemManagerApp/CraftingSystemEditor.svelte b/src/applications/craftingSystemManagerApp/CraftingSystemEditor.svelte index 28616c31..c5cfd6c0 100644 --- a/src/applications/craftingSystemManagerApp/CraftingSystemEditor.svelte +++ b/src/applications/craftingSystemManagerApp/CraftingSystemEditor.svelte @@ -22,9 +22,11 @@ import {EssencesStore} from "../stores/EssenceStore"; import EssenceEditorComponent from "./essenceManager/EssenceEditorComponent.svelte"; import {EssenceEditor} from "./essenceManager/EssenceEditor"; + import type {LocalizationService} from "../common/LocalizationService"; + import type {FabricateAPI} from "../../scripts/api/FabricateAPI"; - export let localization; - export let fabricateAPI; + export let localization: LocalizationService; + export let fabricateAPI: FabricateAPI; const localizationPath = `${Properties.module.id}.CraftingSystemManagerApp` @@ -32,18 +34,17 @@ const selectedCraftingSystem = new SelectedCraftingSystemStore({ craftingSystems }); const essences = new EssencesStore({ selectedCraftingSystem, fabricateAPI }); - const essenceEditor = new EssenceEditor({ fabricateAPI, essences, localization }); - const components = new ComponentsStore({ selectedCraftingSystem, fabricateAPI, initialValue: [] }); - const componentEditor = new CraftingComponentEditor({ fabricateAPI, components: components, localization }); - const recipes = new RecipesStore({ selectedCraftingSystem, fabricateAPI }); + + const essenceEditor = new EssenceEditor({ fabricateAPI, essences, localization, components, recipes }); + const componentEditor = new CraftingComponentEditor({ fabricateAPI, components: components, localization, recipes }); const recipeEditor = new RecipeEditor({ fabricateAPI, recipes, components, localization }); const selectedRecipe = new SelectedRecipeStore({recipes: recipes}); const selectedComponent = new SelectedCraftingComponentStore({craftingComponents: components}); - const craftingSystemEditor = new CraftingSystemEditor({fabricateAPI, craftingSystems, localization, components: components }); + const craftingSystemEditor = new CraftingSystemEditor({fabricateAPI, craftingSystems, localization, components, recipes }); setContext(key, { craftingSystems, diff --git a/src/applications/craftingSystemManagerApp/CraftingSystemEditor.ts b/src/applications/craftingSystemManagerApp/CraftingSystemEditor.ts index e77fabc6..5676c256 100644 --- a/src/applications/craftingSystemManagerApp/CraftingSystemEditor.ts +++ b/src/applications/craftingSystemManagerApp/CraftingSystemEditor.ts @@ -5,11 +5,14 @@ import {LocalizationService} from "../common/LocalizationService"; import {FabricateAPI} from "../../scripts/api/FabricateAPI"; import {FabricateExportModel} from "../../scripts/repository/import/FabricateExportModel"; import {Component} from "../../scripts/crafting/component/Component"; +import {ComponentsStore} from "../stores/ComponentsStore"; +import {RecipesStore} from "../stores/RecipesStore"; class CraftingSystemEditor { private readonly _craftingSystems: Writable; - private readonly _components: Writable; + private readonly _components: ComponentsStore; + private readonly _recipes: RecipesStore; private readonly _localization: LocalizationService; private readonly _fabricateAPI: FabricateAPI; @@ -18,16 +21,19 @@ class CraftingSystemEditor { constructor({ craftingSystems, components, + recipes, localization, fabricateAPI, }: { craftingSystems: Writable; - components: Writable; + components: ComponentsStore; + recipes: RecipesStore; localization: LocalizationService; fabricateAPI: FabricateAPI; }) { this._fabricateAPI = fabricateAPI; this._components = components; + this._recipes = recipes; this._craftingSystems = craftingSystems; this._localization = localization; } @@ -50,6 +56,12 @@ class CraftingSystemEditor { this._craftingSystems.update((craftingSystems) => { return craftingSystems.filter(craftingSystem => craftingSystem.id !== craftingSystemToDelete.id); }); + this._components.update((components => { + return components.filter(component => component.craftingSystemId !== craftingSystemToDelete.id); + })); + this._recipes.update((recipes => { + return recipes.filter(recipe => recipe.craftingSystemId !== craftingSystemToDelete.id); + })); } }); } @@ -169,10 +181,6 @@ class CraftingSystemEditor { return updatedCraftingSystem; } - async deleteComponent(component: Component): Promise { - return this._fabricateAPI.components.deleteById(component.id); - } - async saveComponent(craftingComponent: Component): Promise { const updatedComponent = await this._fabricateAPI.components.save(craftingComponent); this._components.update((components) => { diff --git a/src/applications/craftingSystemManagerApp/componentManager/ComponentEditor.svelte b/src/applications/craftingSystemManagerApp/componentManager/ComponentEditor.svelte index 65820a5f..396a6ef6 100644 --- a/src/applications/craftingSystemManagerApp/componentManager/ComponentEditor.svelte +++ b/src/applications/craftingSystemManagerApp/componentManager/ComponentEditor.svelte @@ -19,6 +19,7 @@ import type {CraftingSystem} from "../../../scripts/crafting/system/CraftingSystem"; import {CraftingComponentEditor} from "./CraftingComponentEditor"; import type {Essence} from "../../../scripts/crafting/essence/Essence"; + import {ComponentsStore} from "../../stores/ComponentsStore"; const localizationPath = `${Properties.module.id}.CraftingSystemManagerApp.tabs.components`; let selectSalvageTab; @@ -33,7 +34,7 @@ }: { localization: LocalizationService, selectedComponent: Readable, - components: Readable, + components: ComponentsStore, selectedCraftingSystem: Readable, componentEditor: CraftingComponentEditor, essences: Readable, diff --git a/src/applications/craftingSystemManagerApp/componentManager/CraftingComponentEditor.ts b/src/applications/craftingSystemManagerApp/componentManager/CraftingComponentEditor.ts index bceac1a4..716338d6 100644 --- a/src/applications/craftingSystemManagerApp/componentManager/CraftingComponentEditor.ts +++ b/src/applications/craftingSystemManagerApp/componentManager/CraftingComponentEditor.ts @@ -6,26 +6,31 @@ import {LocalizationService} from "../../common/LocalizationService"; import {CraftingSystem} from "../../../scripts/crafting/system/CraftingSystem"; import {FabricateAPI} from "../../../scripts/api/FabricateAPI"; import {ComponentsStore} from "../../stores/ComponentsStore"; +import {RecipesStore} from "../../stores/RecipesStore"; class CraftingComponentEditor { private readonly _fabricateAPI: FabricateAPI; private readonly _components: ComponentsStore; + private readonly _recipes: RecipesStore; private readonly _localization: LocalizationService; private readonly _localizationPath = `${Properties.module.id}.CraftingSystemManagerApp.tabs.components`; constructor({ localization, fabricateAPI, - components + components, + recipes }: { localization: LocalizationService; fabricateAPI: FabricateAPI; components: ComponentsStore; + recipes: RecipesStore; }) { this._localization = localization; this._fabricateAPI = fabricateAPI; this._components = components; + this._recipes = recipes; } public async importComponent(event: any, selectedSystem: CraftingSystem) { @@ -110,7 +115,17 @@ class CraftingComponentEditor { return undefined; } const deletedComponent = await this._fabricateAPI.components.deleteById(component.id); - this._components.remove(deletedComponent); + const modifiedComponents = await this._fabricateAPI.components.removeSalvageReferences(component.id, component.craftingSystemId); + const modifiedComponentsById = new Map(modifiedComponents.map(component => [component.id, component])); + const modifiedRecipes = await this._fabricateAPI.recipes.removeComponentReferences(component.id, component.craftingSystemId); + const modifiedRecipesById = new Map(modifiedRecipes.map(recipe => [recipe.id, recipe])); + this._components.update((components) => { + return components.filter(component => component.id !== deletedComponent.id) + .map(component => modifiedComponentsById.get(component.id) || component); + }); + this._recipes.update((recipes) => { + return recipes.map(recipe => modifiedRecipesById.get(recipe.id) || recipe); + }); } public async saveComponent(craftingComponent: Component): Promise { diff --git a/src/applications/craftingSystemManagerApp/essenceManager/EssenceEditor.ts b/src/applications/craftingSystemManagerApp/essenceManager/EssenceEditor.ts index da90c0e2..2d8ff3e1 100644 --- a/src/applications/craftingSystemManagerApp/essenceManager/EssenceEditor.ts +++ b/src/applications/craftingSystemManagerApp/essenceManager/EssenceEditor.ts @@ -6,10 +6,14 @@ import {DefaultDocumentManager} from "../../../scripts/foundry/DocumentManager"; import Properties from "../../../scripts/Properties"; import {FabricateAPI} from "../../../scripts/api/FabricateAPI"; import {EssencesStore} from "../../stores/EssenceStore"; +import {ComponentsStore} from "../../stores/ComponentsStore"; +import {RecipesStore} from "../../stores/RecipesStore"; class EssenceEditor { private readonly _essences: EssencesStore; + private readonly _components: ComponentsStore; + private readonly _recipes: RecipesStore; private readonly _fabricateAPI: FabricateAPI; private readonly _localization: LocalizationService; @@ -17,14 +21,20 @@ class EssenceEditor { constructor({ essences, + components, + recipes, fabricateAPI, localization, }: { essences: EssencesStore; + components: ComponentsStore; + recipes: RecipesStore; fabricateAPI: FabricateAPI; localization: LocalizationService; }) { this._essences = essences; + this._components = components; + this._recipes = recipes; this._fabricateAPI = fabricateAPI; this._localization = localization; } @@ -60,18 +70,35 @@ class EssenceEditor { if (!doDelete) { return; } - await this._fabricateAPI.essences.deleteById(essence.id); - this._essences.remove(essence); + const deletedEssence = await this._fabricateAPI.essences.deleteById(essence.id); + const modifiedComponents = await this._fabricateAPI.components.removeEssenceReferences(essence.id, essence.craftingSystemId); + const modifiedComponentsById = new Map(modifiedComponents.map(component => [component.id, component])); + const modifiedRecipes = await this._fabricateAPI.recipes.removeEssenceReferences(essence.id, essence.craftingSystemId); + const modifiedRecipesById = new Map(modifiedRecipes.map(recipe => [recipe.id, recipe])); + this._essences.remove(deletedEssence); + this._components.update((components) => { + return components.map(component => modifiedComponentsById.get(component.id) || component); + }); + this._recipes.update((recipes) => { + return recipes.map(recipe => modifiedRecipesById.get(recipe.id) || recipe); + }); } public async setActiveEffectSource(event: any, essence: Essence) { const dropEventParser = new DropEventParser({ localizationService: this._localization, - documentManager: new DefaultDocumentManager(), - partType: this._localization.localize(`${Properties.module.id}.typeNames.activeEffectSource.singular`) + documentManager: new DefaultDocumentManager() }) - const dropData = await dropEventParser.parse(event); - essence.activeEffectSource = dropData.itemData; + const dropEvent = await dropEventParser.parse(event); + if (dropEvent.type === "Unknown") { + return; + } + if (dropEvent.type === "Compendium") { + return; + } + if (dropEvent.type === "Item") { + essence.activeEffectSource = dropEvent.data.item; + } await this._fabricateAPI.essences.save(essence); this._essences.insert(essence); return essence; diff --git a/test/RecipeAPI.test.ts b/test/RecipeAPI.test.ts index f565276f..0cc61ad6 100644 --- a/test/RecipeAPI.test.ts +++ b/test/RecipeAPI.test.ts @@ -41,6 +41,7 @@ import { } from "../src/scripts/foundry/DocumentManager"; import Properties from "../src/scripts/Properties"; import {StubRecipeValidator} from "./stubs/StubRecipeValidator"; +import {StubGameProvider} from "./stubs/foundry/StubGameProvider"; const identityFactory = new StubIdentityFactory(); const localizationService = new StubLocalizationService(); @@ -172,7 +173,9 @@ describe("Create", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory("3456abcd") + identityFactory: new StubIdentityFactory("3456abcd"), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const result = await underTest.create({ itemUuid, craftingSystemId }); @@ -197,7 +200,9 @@ describe("Create", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const itemUuid = "1234abcd"; @@ -224,7 +229,9 @@ describe("Create", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const itemUuid = "1234abcd"; @@ -248,7 +255,9 @@ describe("Create", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const result = await underTest.save(testRecipeOne); @@ -271,7 +280,9 @@ describe("Create", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const modified = new DefaultRecipe({ @@ -302,7 +313,9 @@ describe("Create", () => { localizationService, recipeValidator: new StubRecipeValidator(false), recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); expect.assertions(1); @@ -328,7 +341,9 @@ describe("Access", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const result = await underTest.getById("notAValidId"); @@ -351,7 +366,9 @@ describe("Access", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const result = await underTest.getById(testRecipeOne.id); @@ -387,7 +404,9 @@ describe("Access", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const result = await underTest.getAllById([ testRecipeOne.id, testRecipeTwo.id, testRecipeThree.id, "notAValidId" ]); @@ -421,7 +440,9 @@ describe("Access", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const result = await underTest.getAll(); @@ -453,7 +474,9 @@ describe("Access", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const result = await underTest.getAllByCraftingSystemId(testCraftingSystemOne.id); @@ -485,7 +508,9 @@ describe("Access", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const result = await underTest.getAllByItemUuid(testRecipeOne.itemUuid); @@ -518,7 +543,9 @@ describe("Edit", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const result = await underTest.cloneById(testRecipeOne.id); @@ -545,7 +572,9 @@ describe("Edit", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const recipeToEdit = await underTest.getById(testRecipeOne.id); @@ -590,7 +619,9 @@ describe("Delete", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const before = await underTest.getById(testRecipeOne.id); @@ -627,7 +658,9 @@ describe("Delete", () => { essenceAPI: essenceAPI }), recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const before = await underTest.getById(testRecipeOne.id); @@ -659,7 +692,9 @@ describe("Delete", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const before = await underTest.getById(testRecipeOne.id); @@ -693,11 +728,13 @@ describe("Delete", () => { localizationService, recipeValidator: new DefaultRecipeValidator({ craftingSystemAPI: emptyCraftingSystemApi, - componentAPI: componentAPI, - essenceAPI: essenceAPI + essenceAPI: essenceAPI, + componentAPI: componentAPI }), recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const before = await underTest.getById(testRecipeOne.id); @@ -729,7 +766,9 @@ describe("Delete", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const before = await underTest.getById(testRecipeOne.id); @@ -761,7 +800,9 @@ describe("Delete", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const componentIdToDelete = testComponentThree.id; @@ -797,7 +838,9 @@ describe("Delete", () => { localizationService, recipeValidator, recipeStore: recipeDataStore, - identityFactory: new StubIdentityFactory() + identityFactory: new StubIdentityFactory(), + documentManager: new StubDocumentManager(), + gameProvider: new StubGameProvider() }); const essenceIdToDelete = elementalFire.id; diff --git a/test/stubs/api/StubComponentAPI.ts b/test/stubs/api/StubComponentAPI.ts index 442dccff..fc80443a 100644 --- a/test/stubs/api/StubComponentAPI.ts +++ b/test/stubs/api/StubComponentAPI.ts @@ -17,6 +17,13 @@ class StubComponentAPI implements ComponentAPI { this._valuesById = valuesById; } + createMany(_options: { itemUuids: string[]; craftingSystemId: string; componentOptionsByItemUuid?: Map; }): Promise { + throw new Error("Method not implemented."); + } + importCompendium(_options: { craftingSystemId: string; compendiumId: string; }): Promise { + throw new Error("Method not implemented."); + } + get notifications(): NotificationService { throw new Error("This is a stub. Stubs do not provide user interface notifications. "); }