From cc98d5f8d1a6a47834ba15f3649decc307851627 Mon Sep 17 00:00:00 2001 From: Tanner Barlow Date: Mon, 15 Apr 2019 08:05:00 -0700 Subject: [PATCH 01/16] Project service functions --- src/services/projectService.ts | 78 +++++++++++++++++++++++++++++++++- 1 file changed, 77 insertions(+), 1 deletion(-) diff --git a/src/services/projectService.ts b/src/services/projectService.ts index 3b868cf27f..feb1b6e325 100644 --- a/src/services/projectService.ts +++ b/src/services/projectService.ts @@ -1,12 +1,14 @@ import _ from "lodash"; import shortid from "shortid"; import { StorageProviderFactory } from "../providers/storage/storageProviderFactory"; -import { IProject, ISecurityToken, AppError, ErrorCode, AssetState } from "../models/applicationState"; +import { IProject, ISecurityToken, AppError, ErrorCode, AssetState, IAssetMetadata } from "../models/applicationState"; import Guard from "../common/guard"; import { constants } from "../common/constants"; import { ExportProviderFactory } from "../providers/export/exportProviderFactory"; import { decryptProject, encryptProject } from "../common/utils"; import packageJson from "../../package.json"; +import { AssetService } from "./assetService"; +import { forEachAsync } from "../common/extensions/array"; /** * Functions required for a project service @@ -18,6 +20,8 @@ export interface IProjectService { save(project: IProject, securityToken: ISecurityToken): Promise; delete(project: IProject): Promise; isDuplicate(project: IProject, projectList: IProject[]): boolean; + deleteTag(project: IProject, tagName: string, currentAsset: IAssetMetadata): Promise; + renameTag(project: IProject, tagName: string, newTagName: string, currentAsset: IAssetMetadata): Promise; } /** @@ -100,6 +104,78 @@ export default class ProjectService implements IProjectService { return (duplicateProjects !== undefined); } + /** + * Delete a tag from a project, including from all asset metadata files + * @param project The project containing tag to delete + * @param tagName Name of tag to delete + * @param currentAsset Current asset being viewed. Makes changes and returns updated asset to avoid + * needing to reload the asset in the editor page + */ + public async deleteTag(project: IProject, tagName: string, currentAsset: IAssetMetadata): Promise { + const transformer = (tags) => tags.filter((t) => t!== tagName); + return await this.updateProjectTags(project, tagName, currentAsset, transformer); + } + + /** + * Rename a tag within a project, including within all asset metadata files + * @param project The project containing tag to rename + * @param tagName Name of tag to rename + * @param currentAsset Current asset being viewed. Makes changes and returns updated asset to avoid + * needing to reload the asset in the editor page + */ + public async renameTag(project: IProject, tagName: string, newTagName: string, currentAsset: IAssetMetadata): Promise { + const transformer = (tags) => tags.map((t) => (t === tagName) ? newTagName : t); + return await this.updateProjectTags(project, tagName, currentAsset, transformer); + } + + /** + * Update tags within project, including within all asset metadata files + * @param project The project containing tags to update + * @param tagName Name of tag to update within project + * @param currentAsset Current asset being viewed. Makes changes and returns updated asset to avoid + * needing to reload the asset in the editor page + * @param transformer Function that accepts array of tags from a region and returns a modified array of tags + */ + private async updateProjectTags(project: IProject, tagName: string, currentAsset: IAssetMetadata, transformer: (tags: string[]) => string[]) { + const assetService = new AssetService(project); + const assetKeys = Object.keys(project.assets); + await assetKeys.forEachAsync(async (assetKey) => { + const asset = project.assets[assetKey]; + if (asset.state !== AssetState.Tagged) { + return; + } + const assetMetadata = await assetService.getAssetMetadata(asset); + const updatedAssetMetadata = this.updateTagInAssetMetadata(assetMetadata, tagName, transformer); + if (updatedAssetMetadata) { + await assetService.save(updatedAssetMetadata); + } + }); + return this.updateTagInAssetMetadata(currentAsset, tagName, transformer); + } + + /** + * Update tag within asset metadata object + * @param assetMetadata Asset metadata to update + * @param tagName Name of tag being updated + * @param transformer Function that accepts array of tags from a region and returns a modified array of tags + * @returns Modified asset metadata object or null if object does not need to be modified + */ + private updateTagInAssetMetadata(assetMetadata: IAssetMetadata, tagName: string, transformer: (tags: string[]) => string[]) { + let foundTag = false; + for (const region of assetMetadata.regions) { + if (region.tags.find((t) => t === tagName)) { + foundTag = true; + region.tags = transformer(region.tags); + } + } + if (foundTag) { + assetMetadata.regions = assetMetadata.regions.filter((region) => region.tags.length > 0); + assetMetadata.asset.state = (assetMetadata.regions.length) ? AssetState.Tagged : AssetState.Visited; + return assetMetadata; + } + return null; + } + private async saveExportSettings(project: IProject): Promise { if (!project.exportFormat || !project.exportFormat.providerType) { return Promise.resolve(); From 94df15d82569fb6454e7a7bd3dd3834dccf9a9a9 Mon Sep 17 00:00:00 2001 From: Tanner Barlow Date: Mon, 15 Apr 2019 08:31:18 -0700 Subject: [PATCH 02/16] Added tests for project service --- package-lock.json | 20 ++++-- src/services/projectService.test.ts | 102 ++++++++++++++++++++++++++-- src/services/projectService.ts | 39 +++++++++-- 3 files changed, 143 insertions(+), 18 deletions(-) diff --git a/package-lock.json b/package-lock.json index d633f4d85a..7a5d34e053 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7282,7 +7282,8 @@ "code-point-at": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/code-point-at/-/code-point-at-1.1.0.tgz", - "integrity": "sha1-DQcLTQQ6W+ozovGkDi7bPZpMz3c=" + "integrity": "sha1-DQcLTQQ6W+ozovGkDi7bPZpMz3c=", + "optional": true }, "concat-map": { "version": "0.0.1", @@ -7293,7 +7294,8 @@ "console-control-strings": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/console-control-strings/-/console-control-strings-1.1.0.tgz", - "integrity": "sha1-PXz0Rk22RG6mRL9LOVB/mFEAjo4=" + "integrity": "sha1-PXz0Rk22RG6mRL9LOVB/mFEAjo4=", + "optional": true }, "core-util-is": { "version": "1.0.2", @@ -7423,6 +7425,7 @@ "version": "1.0.0", "resolved": "https://registry.npmjs.org/is-fullwidth-code-point/-/is-fullwidth-code-point-1.0.0.tgz", "integrity": "sha1-754xOG8DGn8NZDr4L95QxFfvAMs=", + "optional": true, "requires": { "number-is-nan": "^1.0.0" } @@ -7552,7 +7555,8 @@ "number-is-nan": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/number-is-nan/-/number-is-nan-1.0.1.tgz", - "integrity": "sha1-CXtgK1NCKlIsGvuHkDGDNpQaAR0=" + "integrity": "sha1-CXtgK1NCKlIsGvuHkDGDNpQaAR0=", + "optional": true }, "object-assign": { "version": "4.1.1", @@ -7650,7 +7654,8 @@ "safe-buffer": { "version": "5.1.1", "resolved": "https://registry.npmjs.org/safe-buffer/-/safe-buffer-5.1.1.tgz", - "integrity": "sha512-kKvNJn6Mm93gAczWVJg7wH+wGYWNrDHdWvpUmHyEsgCtIwwo3bqPtV4tR5tuPaUhTOo/kvhVwd8XwwOllGYkbg==" + "integrity": "sha512-kKvNJn6Mm93gAczWVJg7wH+wGYWNrDHdWvpUmHyEsgCtIwwo3bqPtV4tR5tuPaUhTOo/kvhVwd8XwwOllGYkbg==", + "optional": true }, "safer-buffer": { "version": "2.1.2", @@ -7686,6 +7691,7 @@ "version": "1.0.2", "resolved": "https://registry.npmjs.org/string-width/-/string-width-1.0.2.tgz", "integrity": "sha1-EYvfW4zcUaKn5w0hHgfisLmxB9M=", + "optional": true, "requires": { "code-point-at": "^1.0.0", "is-fullwidth-code-point": "^1.0.0", @@ -7748,12 +7754,14 @@ "wrappy": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz", - "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=" + "integrity": "sha1-tSQ9jz7BqjXxNkYFvA0QNuMKtp8=", + "optional": true }, "yallist": { "version": "3.0.2", "resolved": "https://registry.npmjs.org/yallist/-/yallist-3.0.2.tgz", - "integrity": "sha1-hFK0u36Dx8GI2AQcGoN8dz1ti7k=" + "integrity": "sha1-hFK0u36Dx8GI2AQcGoN8dz1ti7k=", + "optional": true } } }, diff --git a/src/services/projectService.test.ts b/src/services/projectService.test.ts index f610e750a3..7faa1fd180 100644 --- a/src/services/projectService.test.ts +++ b/src/services/projectService.test.ts @@ -2,11 +2,13 @@ import _ from "lodash"; import ProjectService, { IProjectService } from "./projectService"; import MockFactory from "../common/mockFactory"; import { StorageProviderFactory } from "../providers/storage/storageProviderFactory"; -import { IProject, IExportFormat, ISecurityToken, AssetState } from "../models/applicationState"; +import { IProject, IExportFormat, ISecurityToken, + AssetState, IAsset, IAssetMetadata } from "../models/applicationState"; import { constants } from "../common/constants"; import { ExportProviderFactory } from "../providers/export/exportProviderFactory"; import { generateKey } from "../common/crypto"; import { encryptProject } from "../common/utils"; +import { AssetService } from "./assetService"; describe("Project Service", () => { let projectSerivce: IProjectService = null; @@ -146,15 +148,105 @@ describe("Project Service", () => { expect(projectSerivce.isDuplicate(testProject, projectList)).toEqual(true); }); - it("deletes all asset metadata files when project is deleted", async () => { - const assets = MockFactory.createTestAssets(10); + function populateProjectAssets(project?: IProject, assetCount = 10) { + if (!project) { + project = MockFactory.createTestProject(); + } + const assets = MockFactory.createTestAssets(assetCount); assets.forEach((asset) => { asset.state = AssetState.Tagged; }); - testProject.assets = _.keyBy(assets, (asset) => asset.id); + project.assets = _.keyBy(assets, (asset) => asset.id); + return project; + } + + it("deletes all asset metadata files when project is deleted", async () => { + const assetCount = 10; + populateProjectAssets(testProject); await projectSerivce.delete(testProject); - expect(storageProviderMock.deleteFile.mock.calls).toHaveLength(assets.length + 1); + expect(storageProviderMock.deleteFile.mock.calls).toHaveLength(assetCount + 1); + }); + + it("Deletes tag from all assets within project", async () => { + const tag1 = "tag1"; + const tag2 = "tag2"; + const region = MockFactory.createTestRegion(undefined, [tag1, tag2]); + const asset: IAsset = { + ...MockFactory.createTestAsset("1"), + state: AssetState.Tagged, + }; + const assetMetadata = MockFactory.createTestAssetMetadata(asset, [region]); + AssetService.prototype.getAssetMetadata = jest.fn((asset: IAsset) => Promise.resolve(assetMetadata)); + + const saveMetadata = jest.fn(); + AssetService.prototype.save = saveMetadata; + + const expectedAssetMetadata: IAssetMetadata = { + ...MockFactory.createTestAssetMetadata( + asset, + [ + { + ...region, + tags: [tag2], + }, + ], + ), + + }; + const project = populateProjectAssets(); + await projectSerivce.deleteTag(project, tag1, assetMetadata); + expect(saveMetadata).toBeCalledWith(expectedAssetMetadata); + }); + + it("Deletes any empty regions after deleting only tag from region", async () => { + const tag1 = "tag1"; + const region = MockFactory.createTestRegion(undefined, [tag1]); + const asset: IAsset = { + ...MockFactory.createTestAsset("1"), + state: AssetState.Tagged, + }; + const assetMetadata = MockFactory.createTestAssetMetadata(asset, [region]); + AssetService.prototype.getAssetMetadata = jest.fn((asset: IAsset) => Promise.resolve(assetMetadata)); + + const saveMetadata = jest.fn(); + AssetService.prototype.save = saveMetadata; + + const expectedAssetMetadata: IAssetMetadata = MockFactory.createTestAssetMetadata(asset, []); + const project = populateProjectAssets(); + await projectSerivce.deleteTag(project, tag1, assetMetadata); + expect(saveMetadata).toBeCalledWith(expectedAssetMetadata); + }); + + it("Updates renamed tag within all assets of project", async () => { + const tag1 = "tag1"; + const newTag = "tag2"; + const region = MockFactory.createTestRegion(undefined, [tag1]); + const asset: IAsset = { + ...MockFactory.createTestAsset("1"), + state: AssetState.Tagged, + }; + const assetMetadata = MockFactory.createTestAssetMetadata(asset, [region]); + AssetService.prototype.getAssetMetadata = jest.fn((asset: IAsset) => Promise.resolve(assetMetadata)); + + const saveMetadata = jest.fn(); + AssetService.prototype.save = saveMetadata; + + const expectedAssetMetadata: IAssetMetadata = { + ...MockFactory.createTestAssetMetadata( + asset, + [ + { + ...region, + tags: [newTag], + }, + ], + ), + + }; + const project = populateProjectAssets(); + await projectSerivce.renameTag(project, tag1, newTag, assetMetadata); + expect(saveMetadata).toBeCalledWith(expectedAssetMetadata); }); }); diff --git a/src/services/projectService.ts b/src/services/projectService.ts index feb1b6e325..3fc0cf01ad 100644 --- a/src/services/projectService.ts +++ b/src/services/projectService.ts @@ -20,8 +20,10 @@ export interface IProjectService { save(project: IProject, securityToken: ISecurityToken): Promise; delete(project: IProject): Promise; isDuplicate(project: IProject, projectList: IProject[]): boolean; - deleteTag(project: IProject, tagName: string, currentAsset: IAssetMetadata): Promise; - renameTag(project: IProject, tagName: string, newTagName: string, currentAsset: IAssetMetadata): Promise; + deleteTag(project: IProject, tagName: string, + currentAsset: IAssetMetadata): Promise; + renameTag(project: IProject, tagName: string, newTagName: string, + currentAsset: IAssetMetadata): Promise; } /** @@ -111,8 +113,9 @@ export default class ProjectService implements IProjectService { * @param currentAsset Current asset being viewed. Makes changes and returns updated asset to avoid * needing to reload the asset in the editor page */ - public async deleteTag(project: IProject, tagName: string, currentAsset: IAssetMetadata): Promise { - const transformer = (tags) => tags.filter((t) => t!== tagName); + public async deleteTag(project: IProject, tagName: string, + currentAsset: IAssetMetadata): Promise { + const transformer = (tags) => tags.filter((t) => t !== tagName); return await this.updateProjectTags(project, tagName, currentAsset, transformer); } @@ -123,7 +126,8 @@ export default class ProjectService implements IProjectService { * @param currentAsset Current asset being viewed. Makes changes and returns updated asset to avoid * needing to reload the asset in the editor page */ - public async renameTag(project: IProject, tagName: string, newTagName: string, currentAsset: IAssetMetadata): Promise { + public async renameTag(project: IProject, tagName: string, newTagName: string, + currentAsset: IAssetMetadata): Promise { const transformer = (tags) => tags.map((t) => (t === tagName) ? newTagName : t); return await this.updateProjectTags(project, tagName, currentAsset, transformer); } @@ -136,9 +140,27 @@ export default class ProjectService implements IProjectService { * needing to reload the asset in the editor page * @param transformer Function that accepts array of tags from a region and returns a modified array of tags */ - private async updateProjectTags(project: IProject, tagName: string, currentAsset: IAssetMetadata, transformer: (tags: string[]) => string[]) { + private async updateProjectTags(project: IProject, tagName: string, currentAsset: IAssetMetadata, + transformer: (tags: string[]) => string[]): Promise { const assetService = new AssetService(project); const assetKeys = Object.keys(project.assets); + // Loop over assets and update if necessary + for (const assetKey of assetKeys) { + const asset = project.assets[assetKey]; + if (asset.state !== AssetState.Tagged) { + return; + } + const assetMetadata = await assetService.getAssetMetadata(asset); + const updatedAssetMetadata = this.updateTagInAssetMetadata(assetMetadata, tagName, transformer); + if (updatedAssetMetadata) { + await assetService.save(updatedAssetMetadata); + } + } + /* + TODO: Replace with async + + For some reason in tests, the `forEachAsync` is not recognized as a function + await assetKeys.forEachAsync(async (assetKey) => { const asset = project.assets[assetKey]; if (asset.state !== AssetState.Tagged) { @@ -150,6 +172,8 @@ export default class ProjectService implements IProjectService { await assetService.save(updatedAssetMetadata); } }); + + */ return this.updateTagInAssetMetadata(currentAsset, tagName, transformer); } @@ -160,7 +184,8 @@ export default class ProjectService implements IProjectService { * @param transformer Function that accepts array of tags from a region and returns a modified array of tags * @returns Modified asset metadata object or null if object does not need to be modified */ - private updateTagInAssetMetadata(assetMetadata: IAssetMetadata, tagName: string, transformer: (tags: string[]) => string[]) { + private updateTagInAssetMetadata(assetMetadata: IAssetMetadata, tagName: string, + transformer: (tags: string[]) => string[]): IAssetMetadata { let foundTag = false; for (const region of assetMetadata.regions) { if (region.tags.find((t) => t === tagName)) { From 295dcadff2b45ccd0724115a406d47e8b762630f Mon Sep 17 00:00:00 2001 From: Tanner Barlow Date: Mon, 15 Apr 2019 09:06:03 -0700 Subject: [PATCH 03/16] Added confirm and strings for tag and rename operations --- src/common/localization/en-us.ts | 8 +++++ src/common/localization/es-cl.ts | 8 +++++ src/common/mockFactory.ts | 2 ++ src/common/strings.ts | 8 +++++ .../components/common/tagInput/tagInput.tsx | 21 +++++++++---- .../pages/editorPage/editorPage.tsx | 31 +++++++++++++++++++ 6 files changed, 72 insertions(+), 6 deletions(-) diff --git a/src/common/localization/en-us.ts b/src/common/localization/en-us.ts index c9074a6307..c4f7c2daa5 100644 --- a/src/common/localization/en-us.ts +++ b/src/common/localization/en-us.ts @@ -256,6 +256,14 @@ export const english: IAppStrings = { apply: "Apply Tag with Hot Key", lock: "Lock Tag with Hot Key", }, + rename: { + title: "Rename Tag", + confirmation: "Are you sure you want to rename this tag? It will be renamed throughout all assets", + }, + delete: { + title: "Delete Tag", + confirmation: "Are you sure you want to delete this tag? It will be deleted throughout all assets and any regions where this is the only tag will also be deleted", + } }, canvas: { removeAllRegions: { diff --git a/src/common/localization/es-cl.ts b/src/common/localization/es-cl.ts index 1e9d3cd396..fcabf7b8ba 100644 --- a/src/common/localization/es-cl.ts +++ b/src/common/localization/es-cl.ts @@ -258,6 +258,14 @@ export const spanish: IAppStrings = { apply: "Aplicar etiqueta con tecla de acceso rápido", lock: "Bloquear etiqueta con tecla de acceso rápido", }, + rename: { + title: "Cambiar el nombre de la etiqueta", + confirmation: "¿Está seguro que quiere cambiar el nombre de esta etiqueta? Será cambiada en todos los activos", + }, + delete: { + title: "Delete Tag", + confirmation: "¿Está seguro que quiere borrar esta etiqueta? Será borrada en todos los activos y en las regiones donde esta etiqueta sea la única, la region también será borrada", + } }, canvas: { removeAllRegions: { diff --git a/src/common/mockFactory.ts b/src/common/mockFactory.ts index 1db2f08d61..60a2e6e7b5 100644 --- a/src/common/mockFactory.ts +++ b/src/common/mockFactory.ts @@ -802,6 +802,8 @@ export default class MockFactory { save: jest.fn((project: IProject) => Promise.resolve(project)), delete: jest.fn((project: IProject) => Promise.resolve()), isDuplicate: jest.fn((project: IProject, projectList: IProject[]) => true), + renameTag: jest.fn(), + deleteTag: jest.fn(), }; } diff --git a/src/common/strings.ts b/src/common/strings.ts index 31e0114d75..badf7908d0 100644 --- a/src/common/strings.ts +++ b/src/common/strings.ts @@ -255,6 +255,14 @@ export interface IAppStrings { apply: string; lock: string; }, + rename: { + title: string; + confirmation: string; + }, + delete: { + title: string; + confirmation: string; + } } canvas: { removeAllRegions: { diff --git a/src/react/components/common/tagInput/tagInput.tsx b/src/react/components/common/tagInput/tagInput.tsx index 7291ad16f9..b1430c4ca0 100644 --- a/src/react/components/common/tagInput/tagInput.tsx +++ b/src/react/components/common/tagInput/tagInput.tsx @@ -31,9 +31,9 @@ export interface ITagInputProps { /** Function to call on clicking individual tag while holding CTRL key */ onCtrlTagClick?: (tag: ITag) => void; /** Function to call when tag is renamed */ - onTagRenamed?: (oldTag: string, newTag: string) => void; + onTagRenamed?: (tagName: string, newTagName: string) => void; /** Function to call when tag is deleted */ - onTagDeleted?: (tag: ITag) => void; + onTagDeleted?: (tagName: string) => void; /** Always show tag input box */ showTagInputBox?: boolean; /** Always show tag search box */ @@ -219,20 +219,25 @@ export class TagInput extends React.Component { }, () => this.props.onChange(tags)); } - private updateTag = (oldTag: ITag, newTag: ITag) => { - if (oldTag === newTag) { + private updateTag = (tag: ITag, newTag: ITag) => { + if (tag === newTag) { return; } if (!newTag.name.length) { toast.warn(strings.tags.warnings.emptyName); return; } - if (newTag.name !== oldTag.name && this.state.tags.some((t) => t.name === newTag.name)) { + const nameChange = tag.name !== newTag.name; + if (nameChange && this.state.tags.some((t) => t.name === newTag.name)) { toast.warn(strings.tags.warnings.existingName); return; } + if (nameChange && this.props.onTagRenamed) { + this.props.onTagRenamed(tag.name, newTag.name); + return; + } const tags = this.state.tags.map((t) => { - return (t.name === oldTag.name) ? newTag : t; + return (t.name === tag.name) ? newTag : t; }); this.setState({ tags, @@ -389,6 +394,10 @@ export class TagInput extends React.Component { if (!tag) { return; } + if (this.props.onTagDeleted) { + this.props.onTagDeleted(tag.name); + return; + } const index = this.state.tags.indexOf(tag); const tags = this.state.tags.filter((t) => t.name !== tag.name); this.setState({ diff --git a/src/react/components/pages/editorPage/editorPage.tsx b/src/react/components/pages/editorPage/editorPage.tsx index 86d4719838..650930f41a 100644 --- a/src/react/components/pages/editorPage/editorPage.tsx +++ b/src/react/components/pages/editorPage/editorPage.tsx @@ -28,6 +28,7 @@ import "./editorPage.scss"; import EditorSideBar from "./editorSideBar"; import { EditorToolbar } from "./editorToolbar"; import Alert from "../../common/alert/alert"; +import Confirm from "../../common/confirm/confirm"; // tslint:disable-next-line:no-var-requires const tagColors = require("../../common/tagColors.json"); @@ -120,6 +121,8 @@ export default class EditorPage extends React.Component = React.createRef(); + private renameTagConfirm: React.RefObject = React.createRef(); + private deleteTagConfirm: React.RefObject = React.createRef(); public async componentDidMount() { const projectId = this.props.match.params["projectId"]; @@ -232,8 +235,20 @@ export default class EditorPage extends React.Component + + this.canvas.current.applyTag(tag.name)); } + private confirmTagRenamed = (tagName: string, newTagName: string): void => { + this.renameTagConfirm.current.open(tagName, newTagName); + } + + private onTagRenamed = (tagName: string, newTagName: string): void => { + debugger; + } + + private confirmTagDeleted = (tagName: string): void => { + this.deleteTagConfirm.current.open(tagName); + } + + private onTagDeleted = (tagName: string): void => { + debugger; + } + private onCtrlTagClicked = (tag: ITag): void => { const locked = this.state.lockedTags; this.setState({ From 2f6ff66b33c286039cdf3e297c50dc046b1c94ad Mon Sep 17 00:00:00 2001 From: Tanner Barlow Date: Mon, 15 Apr 2019 10:28:22 -0700 Subject: [PATCH 04/16] Split out project functions and asset functions into respective services --- src/common/localization/en-us.ts | 8 +- src/common/localization/es-cl.ts | 8 +- src/common/strings.ts | 2 +- .../pages/editorPage/editorPage.tsx | 33 ++++- src/services/assetService.test.ts | 104 ++++++++++++++- src/services/assetService.ts | 99 +++++++++++++++ src/services/projectService.test.ts | 119 +++++------------- src/services/projectService.ts | 107 ++-------------- 8 files changed, 281 insertions(+), 199 deletions(-) diff --git a/src/common/localization/en-us.ts b/src/common/localization/en-us.ts index c4f7c2daa5..2a07e1e715 100644 --- a/src/common/localization/en-us.ts +++ b/src/common/localization/en-us.ts @@ -262,8 +262,9 @@ export const english: IAppStrings = { }, delete: { title: "Delete Tag", - confirmation: "Are you sure you want to delete this tag? It will be deleted throughout all assets and any regions where this is the only tag will also be deleted", - } + confirmation: "Are you sure you want to delete this tag? It will be deleted throughout all assets \ + and any regions where this is the only tag will also be deleted", + }, }, canvas: { removeAllRegions: { @@ -275,7 +276,8 @@ export const english: IAppStrings = { enforceTaggedRegions: { title: "Invalid region(s) detected", // tslint:disable-next-line:max-line-length - description: "1 or more regions have not been tagged. Ensure all regions are tagged before continuing to next asset.", + description: "1 or more regions have not been tagged. Ensure all regions are tagged before \ + continuing to next asset.", }, }, }, diff --git a/src/common/localization/es-cl.ts b/src/common/localization/es-cl.ts index fcabf7b8ba..41315013db 100644 --- a/src/common/localization/es-cl.ts +++ b/src/common/localization/es-cl.ts @@ -260,12 +260,14 @@ export const spanish: IAppStrings = { }, rename: { title: "Cambiar el nombre de la etiqueta", - confirmation: "¿Está seguro que quiere cambiar el nombre de esta etiqueta? Será cambiada en todos los activos", + confirmation: "¿Está seguro que quiere cambiar el nombre de esta etiqueta? \ + Será cambiada en todos los activos", }, delete: { title: "Delete Tag", - confirmation: "¿Está seguro que quiere borrar esta etiqueta? Será borrada en todos los activos y en las regiones donde esta etiqueta sea la única, la region también será borrada", - } + confirmation: "¿Está seguro que quiere borrar esta etiqueta? Será borrada en todos \ + los activos y en las regiones donde esta etiqueta sea la única, la region también será borrada", + }, }, canvas: { removeAllRegions: { diff --git a/src/common/strings.ts b/src/common/strings.ts index badf7908d0..7a842c0949 100644 --- a/src/common/strings.ts +++ b/src/common/strings.ts @@ -262,7 +262,7 @@ export interface IAppStrings { delete: { title: string; confirmation: string; - } + }, } canvas: { removeAllRegions: { diff --git a/src/react/components/pages/editorPage/editorPage.tsx b/src/react/components/pages/editorPage/editorPage.tsx index 650930f41a..dcbf10f08b 100644 --- a/src/react/components/pages/editorPage/editorPage.tsx +++ b/src/react/components/pages/editorPage/editorPage.tsx @@ -29,6 +29,7 @@ import EditorSideBar from "./editorSideBar"; import { EditorToolbar } from "./editorToolbar"; import Alert from "../../common/alert/alert"; import Confirm from "../../common/confirm/confirm"; +import ProjectService from "../../../../services/projectService"; // tslint:disable-next-line:no-var-requires const tagColors = require("../../common/tagColors.json"); @@ -228,7 +229,7 @@ export default class EditorPage extends React.Component
{ - debugger; + private onTagRenamed = async (tagName: string, newTagName: string): Promise => { + const { project, selectedAsset } = this.state; + const assetService = new AssetService(project); + const asset = await assetService.renameTag(project.assets, tagName, newTagName, selectedAsset); + + const projectService = new ProjectService(); + const newProject = projectService.renameTag(project, tagName, newTagName); + await this.props.actions.saveProject(newProject); + + this.setState({ + selectedAsset: asset, + project: newProject, + }); } private confirmTagDeleted = (tagName: string): void => { this.deleteTagConfirm.current.open(tagName); } - private onTagDeleted = (tagName: string): void => { - debugger; + private onTagDeleted = async (tagName: string): Promise => { + const { project, selectedAsset } = this.state; + const assetService = new AssetService(project); + const asset = await assetService.deleteTag(project.assets, tagName, selectedAsset); + + const projectService = new ProjectService(); + const newProject = projectService.deleteTag(project, tagName); + await this.props.actions.saveProject(newProject); + + this.setState({ + selectedAsset: asset, + project: newProject, + }); } private onCtrlTagClicked = (tag: ITag): void => { diff --git a/src/services/assetService.test.ts b/src/services/assetService.test.ts index 90d07d15bd..ef84e11200 100644 --- a/src/services/assetService.test.ts +++ b/src/services/assetService.test.ts @@ -1,5 +1,5 @@ import { AssetService } from "./assetService"; -import { AssetType, IAssetMetadata, AssetState } from "../models/applicationState"; +import { AssetType, IAssetMetadata, AssetState, IAsset, IProject } from "../models/applicationState"; import MockFactory from "../common/mockFactory"; import { AssetProviderFactory, IAssetProvider } from "../providers/storage/assetProviderFactory"; import { StorageProviderFactory } from "../providers/storage/storageProviderFactory"; @@ -7,6 +7,7 @@ import { constants } from "../common/constants"; import { TFRecordsBuilder, FeatureType } from "../providers/export/tensorFlowRecords/tensorFlowBuilder"; import HtmlFileReader from "../common/htmlFileReader"; import { encodeFileURI } from "../common/utils"; +import _ from "lodash"; describe("Asset Service", () => { describe("Static Methods", () => { @@ -323,4 +324,105 @@ describe("Asset Service", () => { expect(Math.floor(result.regions[1].points[1].y)).toEqual(800); }); }); + + describe("Tag Update functions", () => { + + function populateProjectAssets(project?: IProject, assetCount = 10) { + if (!project) { + project = MockFactory.createTestProject(); + } + const assets = MockFactory.createTestAssets(assetCount); + assets.forEach((asset) => { + asset.state = AssetState.Tagged; + }); + + project.assets = _.keyBy(assets, (asset) => asset.id); + return project; + } + + it("Deletes tag from assets", async () => { + const tag1 = "tag1"; + const tag2 = "tag2"; + const region = MockFactory.createTestRegion(undefined, [tag1, tag2]); + const asset: IAsset = { + ...MockFactory.createTestAsset("1"), + state: AssetState.Tagged, + }; + const assetMetadata = MockFactory.createTestAssetMetadata(asset, [region]); + AssetService.prototype.getAssetMetadata = jest.fn((asset: IAsset) => Promise.resolve(assetMetadata)); + + const saveMetadata = jest.fn(); + AssetService.prototype.save = saveMetadata; + + const expectedAssetMetadata: IAssetMetadata = { + ...MockFactory.createTestAssetMetadata( + asset, + [ + { + ...region, + tags: [tag2], + }, + ], + ), + + }; + + const project = populateProjectAssets(); + const assetService = new AssetService(project); + await assetService.deleteTag(project.assets, tag1, assetMetadata); + expect(saveMetadata).toBeCalledWith(expectedAssetMetadata); + }); + + it("Deletes empty regions after deleting only tag from region", async () => { + const tag1 = "tag1"; + const region = MockFactory.createTestRegion(undefined, [tag1]); + const asset: IAsset = { + ...MockFactory.createTestAsset("1"), + state: AssetState.Tagged, + }; + const assetMetadata = MockFactory.createTestAssetMetadata(asset, [region]); + AssetService.prototype.getAssetMetadata = jest.fn((asset: IAsset) => Promise.resolve(assetMetadata)); + + const saveMetadata = jest.fn(); + AssetService.prototype.save = saveMetadata; + + const expectedAssetMetadata: IAssetMetadata = MockFactory.createTestAssetMetadata(asset, []); + const project = populateProjectAssets(); + const assetService = new AssetService(project); + await assetService.deleteTag(project.assets, tag1, assetMetadata); + expect(saveMetadata).toBeCalledWith(expectedAssetMetadata); + }); + + it("Updates renamed tag within all assets", async () => { + const tag1 = "tag1"; + const newTag = "tag2"; + const region = MockFactory.createTestRegion(undefined, [tag1]); + const asset: IAsset = { + ...MockFactory.createTestAsset("1"), + state: AssetState.Tagged, + }; + const assetMetadata = MockFactory.createTestAssetMetadata(asset, [region]); + AssetService.prototype.getAssetMetadata = jest.fn((asset: IAsset) => Promise.resolve(assetMetadata)); + + const saveMetadata = jest.fn(); + AssetService.prototype.save = saveMetadata; + + const expectedAssetMetadata: IAssetMetadata = { + ...MockFactory.createTestAssetMetadata( + asset, + [ + { + ...region, + tags: [newTag], + }, + ], + ), + + }; + const project = populateProjectAssets(); + const assetService = new AssetService(project); + await assetService.renameTag(project.assets, tag1, newTag, assetMetadata); + expect(saveMetadata).toBeCalledWith(expectedAssetMetadata); + }); + }); }); diff --git a/src/services/assetService.ts b/src/services/assetService.ts index 58335b606a..99208ecdca 100644 --- a/src/services/assetService.ts +++ b/src/services/assetService.ts @@ -204,6 +204,105 @@ export class AssetService { } } + /** + * Delete a tag from asset metadata files + * @param assets The assets containing tag to delete + * @param tagName Name of tag to delete + * @param currentAsset Current asset being viewed. Makes changes and returns updated asset to avoid + * needing to reload the asset in the editor page + */ + public async deleteTag(assets: {[id: string]: IAsset}, tagName: string, + currentAsset?: IAssetMetadata): Promise { + const transformer = (tags) => tags.filter((t) => t !== tagName); + return await this.updateAssetTags(assets, tagName, transformer, currentAsset); + } + + /** + * Rename a tag within asset metadata files + * @param assets The assets containing tag to rename + * @param tagName Name of tag to rename + * @param currentAsset Current asset being viewed. Makes changes and returns updated asset to avoid + * needing to reload the asset in the editor page + */ + public async renameTag(assets: {[id: string]: IAsset}, tagName: string, newTagName: string, + currentAsset?: IAssetMetadata): Promise { + const transformer = (tags) => tags.map((t) => (t === tagName) ? newTagName : t); + return await this.updateAssetTags(assets, tagName, transformer, currentAsset); + } + + /** + * Update tags within asset metadata files + * @param assets The assets containing tags to update + * @param tagName Name of tag to update within project + * @param currentAsset Current asset being viewed. Makes changes and returns updated asset to avoid + * needing to reload the asset in the editor page + * @param transformer Function that accepts array of tags from a region and returns a modified array of tags + */ + private async updateAssetTags( + assets: {[id: string]: IAsset}, + tagName: string, + transformer: (tags: string[]) => string[], + currentAsset?: IAssetMetadata): Promise { + const assetKeys = Object.keys(assets); + // Loop over assets and update if necessary + for (const assetKey of assetKeys) { + const asset = assets[assetKey]; + if (asset.state !== AssetState.Tagged) { + return; + } + const assetMetadata = await this.getAssetMetadata(asset); + const updatedAssetMetadata = this.updateTagInAssetMetadata(assetMetadata, tagName, transformer); + if (updatedAssetMetadata) { + await this.save(updatedAssetMetadata); + } + } + if (currentAsset) { + return this.updateTagInAssetMetadata(currentAsset, tagName, transformer); + } + /* + TODO: Replace with async + + For some reason in tests, the `forEachAsync` is not recognized as a function + + await assetKeys.forEachAsync(async (assetKey) => { + const asset = project.assets[assetKey]; + if (asset.state !== AssetState.Tagged) { + return; + } + const assetMetadata = await assetService.getAssetMetadata(asset); + const updatedAssetMetadata = this.updateTagInAssetMetadata(assetMetadata, tagName, transformer); + if (updatedAssetMetadata) { + await assetService.save(updatedAssetMetadata); + } + }); + + */ + } + + /** + * Update tag within asset metadata object + * @param assetMetadata Asset metadata to update + * @param tagName Name of tag being updated + * @param transformer Function that accepts array of tags from a region and returns a modified array of tags + * @returns Modified asset metadata object or null if object does not need to be modified + */ + private updateTagInAssetMetadata(assetMetadata: IAssetMetadata, tagName: string, + transformer: (tags: string[]) => string[]): IAssetMetadata { + let foundTag = false; + for (const region of assetMetadata.regions) { + if (region.tags.find((t) => t === tagName)) { + foundTag = true; + region.tags = transformer(region.tags); + } + } + if (foundTag) { + assetMetadata.regions = assetMetadata.regions.filter((region) => region.tags.length > 0); + assetMetadata.asset.state = (assetMetadata.regions.length) ? AssetState.Tagged : AssetState.Visited; + return assetMetadata; + } + return null; + } + private async getRegionsFromTFRecord(asset: IAsset): Promise { const objectArray = await this.getTFRecordMetadata(asset); const regions: IRegion[] = []; diff --git a/src/services/projectService.test.ts b/src/services/projectService.test.ts index 7faa1fd180..9f5b6278ed 100644 --- a/src/services/projectService.test.ts +++ b/src/services/projectService.test.ts @@ -2,13 +2,11 @@ import _ from "lodash"; import ProjectService, { IProjectService } from "./projectService"; import MockFactory from "../common/mockFactory"; import { StorageProviderFactory } from "../providers/storage/storageProviderFactory"; -import { IProject, IExportFormat, ISecurityToken, - AssetState, IAsset, IAssetMetadata } from "../models/applicationState"; +import { IProject, IExportFormat, ISecurityToken, AssetState } from "../models/applicationState"; import { constants } from "../common/constants"; import { ExportProviderFactory } from "../providers/export/exportProviderFactory"; import { generateKey } from "../common/crypto"; import { encryptProject } from "../common/utils"; -import { AssetService } from "./assetService"; describe("Project Service", () => { let projectSerivce: IProjectService = null; @@ -148,105 +146,44 @@ describe("Project Service", () => { expect(projectSerivce.isDuplicate(testProject, projectList)).toEqual(true); }); - function populateProjectAssets(project?: IProject, assetCount = 10) { - if (!project) { - project = MockFactory.createTestProject(); - } - const assets = MockFactory.createTestAssets(assetCount); + it("deletes all asset metadata files when project is deleted", async () => { + const assets = MockFactory.createTestAssets(10); assets.forEach((asset) => { asset.state = AssetState.Tagged; }); - project.assets = _.keyBy(assets, (asset) => asset.id); - return project; - } - - it("deletes all asset metadata files when project is deleted", async () => { - const assetCount = 10; - populateProjectAssets(testProject); + testProject.assets = _.keyBy(assets, (asset) => asset.id); await projectSerivce.delete(testProject); - expect(storageProviderMock.deleteFile.mock.calls).toHaveLength(assetCount + 1); - }); - - it("Deletes tag from all assets within project", async () => { - const tag1 = "tag1"; - const tag2 = "tag2"; - const region = MockFactory.createTestRegion(undefined, [tag1, tag2]); - const asset: IAsset = { - ...MockFactory.createTestAsset("1"), - state: AssetState.Tagged, - }; - const assetMetadata = MockFactory.createTestAssetMetadata(asset, [region]); - AssetService.prototype.getAssetMetadata = jest.fn((asset: IAsset) => Promise.resolve(assetMetadata)); - - const saveMetadata = jest.fn(); - AssetService.prototype.save = saveMetadata; - - const expectedAssetMetadata: IAssetMetadata = { - ...MockFactory.createTestAssetMetadata( - asset, - [ - { - ...region, - tags: [tag2], - }, - ], - ), - - }; - const project = populateProjectAssets(); - await projectSerivce.deleteTag(project, tag1, assetMetadata); - expect(saveMetadata).toBeCalledWith(expectedAssetMetadata); + expect(storageProviderMock.deleteFile.mock.calls).toHaveLength(assets.length + 1); }); - it("Deletes any empty regions after deleting only tag from region", async () => { - const tag1 = "tag1"; - const region = MockFactory.createTestRegion(undefined, [tag1]); - const asset: IAsset = { - ...MockFactory.createTestAsset("1"), - state: AssetState.Tagged, + it("Deletes a tag from project", () => { + const tag = MockFactory.createTestTag(); + const project: IProject = { + ...MockFactory.createTestProject(), + tags: [tag], }; - const assetMetadata = MockFactory.createTestAssetMetadata(asset, [region]); - AssetService.prototype.getAssetMetadata = jest.fn((asset: IAsset) => Promise.resolve(assetMetadata)); - - const saveMetadata = jest.fn(); - AssetService.prototype.save = saveMetadata; - - const expectedAssetMetadata: IAssetMetadata = MockFactory.createTestAssetMetadata(asset, []); - const project = populateProjectAssets(); - await projectSerivce.deleteTag(project, tag1, assetMetadata); - expect(saveMetadata).toBeCalledWith(expectedAssetMetadata); + const projectService = new ProjectService(); + expect(projectSerivce.deleteTag(project, tag.name)).toEqual({ + ...project, + tags: [], + }); }); - it("Updates renamed tag within all assets of project", async () => { - const tag1 = "tag1"; - const newTag = "tag2"; - const region = MockFactory.createTestRegion(undefined, [tag1]); - const asset: IAsset = { - ...MockFactory.createTestAsset("1"), - state: AssetState.Tagged, + it("Renames a tag within project", () => { + const tag = MockFactory.createTestTag(); + const project: IProject = { + ...MockFactory.createTestProject(), + tags: [tag], }; - const assetMetadata = MockFactory.createTestAssetMetadata(asset, [region]); - AssetService.prototype.getAssetMetadata = jest.fn((asset: IAsset) => Promise.resolve(assetMetadata)); - - const saveMetadata = jest.fn(); - AssetService.prototype.save = saveMetadata; - - const expectedAssetMetadata: IAssetMetadata = { - ...MockFactory.createTestAssetMetadata( - asset, - [ - { - ...region, - tags: [newTag], - }, - ], - ), - - }; - const project = populateProjectAssets(); - await projectSerivce.renameTag(project, tag1, newTag, assetMetadata); - expect(saveMetadata).toBeCalledWith(expectedAssetMetadata); + const projectService = new ProjectService(); + expect(projectSerivce.renameTag(project, tag.name, "test")).toEqual({ + ...project, + tags: [{ + name: "test", + color: expect.any(String), + }], + }); }); }); diff --git a/src/services/projectService.ts b/src/services/projectService.ts index 3fc0cf01ad..e092c5343d 100644 --- a/src/services/projectService.ts +++ b/src/services/projectService.ts @@ -20,10 +20,8 @@ export interface IProjectService { save(project: IProject, securityToken: ISecurityToken): Promise; delete(project: IProject): Promise; isDuplicate(project: IProject, projectList: IProject[]): boolean; - deleteTag(project: IProject, tagName: string, - currentAsset: IAssetMetadata): Promise; - renameTag(project: IProject, tagName: string, newTagName: string, - currentAsset: IAssetMetadata): Promise; + deleteTag(project: IProject, tagName: string): IProject; + renameTag(project: IProject, tagName: string, newTagName: string); } /** @@ -106,99 +104,18 @@ export default class ProjectService implements IProjectService { return (duplicateProjects !== undefined); } - /** - * Delete a tag from a project, including from all asset metadata files - * @param project The project containing tag to delete - * @param tagName Name of tag to delete - * @param currentAsset Current asset being viewed. Makes changes and returns updated asset to avoid - * needing to reload the asset in the editor page - */ - public async deleteTag(project: IProject, tagName: string, - currentAsset: IAssetMetadata): Promise { - const transformer = (tags) => tags.filter((t) => t !== tagName); - return await this.updateProjectTags(project, tagName, currentAsset, transformer); - } - - /** - * Rename a tag within a project, including within all asset metadata files - * @param project The project containing tag to rename - * @param tagName Name of tag to rename - * @param currentAsset Current asset being viewed. Makes changes and returns updated asset to avoid - * needing to reload the asset in the editor page - */ - public async renameTag(project: IProject, tagName: string, newTagName: string, - currentAsset: IAssetMetadata): Promise { - const transformer = (tags) => tags.map((t) => (t === tagName) ? newTagName : t); - return await this.updateProjectTags(project, tagName, currentAsset, transformer); + public deleteTag(project: IProject, tagName: string): IProject { + return { + ...project, + tags: project.tags.filter((t) => t.name !== tagName), + }; } - /** - * Update tags within project, including within all asset metadata files - * @param project The project containing tags to update - * @param tagName Name of tag to update within project - * @param currentAsset Current asset being viewed. Makes changes and returns updated asset to avoid - * needing to reload the asset in the editor page - * @param transformer Function that accepts array of tags from a region and returns a modified array of tags - */ - private async updateProjectTags(project: IProject, tagName: string, currentAsset: IAssetMetadata, - transformer: (tags: string[]) => string[]): Promise { - const assetService = new AssetService(project); - const assetKeys = Object.keys(project.assets); - // Loop over assets and update if necessary - for (const assetKey of assetKeys) { - const asset = project.assets[assetKey]; - if (asset.state !== AssetState.Tagged) { - return; - } - const assetMetadata = await assetService.getAssetMetadata(asset); - const updatedAssetMetadata = this.updateTagInAssetMetadata(assetMetadata, tagName, transformer); - if (updatedAssetMetadata) { - await assetService.save(updatedAssetMetadata); - } - } - /* - TODO: Replace with async - - For some reason in tests, the `forEachAsync` is not recognized as a function - - await assetKeys.forEachAsync(async (assetKey) => { - const asset = project.assets[assetKey]; - if (asset.state !== AssetState.Tagged) { - return; - } - const assetMetadata = await assetService.getAssetMetadata(asset); - const updatedAssetMetadata = this.updateTagInAssetMetadata(assetMetadata, tagName, transformer); - if (updatedAssetMetadata) { - await assetService.save(updatedAssetMetadata); - } - }); - - */ - return this.updateTagInAssetMetadata(currentAsset, tagName, transformer); - } - - /** - * Update tag within asset metadata object - * @param assetMetadata Asset metadata to update - * @param tagName Name of tag being updated - * @param transformer Function that accepts array of tags from a region and returns a modified array of tags - * @returns Modified asset metadata object or null if object does not need to be modified - */ - private updateTagInAssetMetadata(assetMetadata: IAssetMetadata, tagName: string, - transformer: (tags: string[]) => string[]): IAssetMetadata { - let foundTag = false; - for (const region of assetMetadata.regions) { - if (region.tags.find((t) => t === tagName)) { - foundTag = true; - region.tags = transformer(region.tags); - } - } - if (foundTag) { - assetMetadata.regions = assetMetadata.regions.filter((region) => region.tags.length > 0); - assetMetadata.asset.state = (assetMetadata.regions.length) ? AssetState.Tagged : AssetState.Visited; - return assetMetadata; - } - return null; + public renameTag(project: IProject, tagName: string, newTagName: string): IProject { + return { + ...project, + tags: project.tags.map((t) => (t.name === tagName) ? {...t, name: newTagName} : t), + }; } private async saveExportSettings(project: IProject): Promise { From 0fa8dba2169d93fbd645122aacb0b23b0b12b013 Mon Sep 17 00:00:00 2001 From: Tanner Barlow Date: Mon, 15 Apr 2019 12:43:49 -0700 Subject: [PATCH 05/16] Editor Page and canvas upates --- src/common/mockFactory.ts | 2 - .../components/pages/editorPage/canvas.tsx | 20 +++++++--- .../pages/editorPage/editorPage.tsx | 40 +++++++++++-------- src/services/assetService.ts | 3 ++ src/services/projectService.test.ts | 29 -------------- src/services/projectService.ts | 20 +--------- 6 files changed, 42 insertions(+), 72 deletions(-) diff --git a/src/common/mockFactory.ts b/src/common/mockFactory.ts index 60a2e6e7b5..1db2f08d61 100644 --- a/src/common/mockFactory.ts +++ b/src/common/mockFactory.ts @@ -802,8 +802,6 @@ export default class MockFactory { save: jest.fn((project: IProject) => Promise.resolve(project)), delete: jest.fn((project: IProject) => Promise.resolve()), isDuplicate: jest.fn((project: IProject, projectList: IProject[]) => true), - renameTag: jest.fn(), - deleteTag: jest.fn(), }; } diff --git a/src/react/components/pages/editorPage/canvas.tsx b/src/react/components/pages/editorPage/canvas.tsx index 33d9dc5f6a..e73ce4efbd 100644 --- a/src/react/components/pages/editorPage/canvas.tsx +++ b/src/react/components/pages/editorPage/canvas.tsx @@ -192,12 +192,20 @@ export default class Canvas extends React.Component return this.state.currentAsset.regions.filter((r) => selectedRegions.find((id) => r.id === id)); } - public updateCanvasToolsRegions = (): void => { - for (const region of this.state.currentAsset.regions) { - this.editor.RM.updateTagsById( - region.id, - CanvasHelpers.getTagsDescriptor(this.props.project.tags, region), - ); + public updateCanvasToolsRegions = (asset?: IAssetMetadata): void => { + if (asset) { + this.setState({ + currentAsset: asset, + }); + this.clearAllRegions(); + this.addRegionsToCanvasTools(asset.regions); + } else { + for (const region of this.state.currentAsset.regions) { + this.editor.RM.updateTagsById( + region.id, + CanvasHelpers.getTagsDescriptor(this.props.project.tags, region), + ); + } } } diff --git a/src/react/components/pages/editorPage/editorPage.tsx b/src/react/components/pages/editorPage/editorPage.tsx index dcbf10f08b..bba44a2248 100644 --- a/src/react/components/pages/editorPage/editorPage.tsx +++ b/src/react/components/pages/editorPage/editorPage.tsx @@ -229,7 +229,7 @@ export default class EditorPage extends React.Component
(t.name === tagName) ? {...t, name: newTagName} : t), + } this.setState({ - selectedAsset: asset, project: newProject, + selectedAsset: asset || selectedAsset, + }, async () => { + await this.props.actions.saveProject(newProject); + if (asset) { + this.canvas.current.updateCanvasToolsRegions(asset); + } }); } @@ -328,18 +333,21 @@ export default class EditorPage extends React.Component => { - const { project, selectedAsset } = this.state; - const assetService = new AssetService(project); - const asset = await assetService.deleteTag(project.assets, tagName, selectedAsset); - - const projectService = new ProjectService(); - const newProject = projectService.deleteTag(project, tagName); + const { selectedAsset } = this.state; + const { project } = this.props; + const newProject: IProject = { + ...project, + tags: project.tags.filter((t) => t.name !== tagName) + } await this.props.actions.saveProject(newProject); + debugger; - this.setState({ - selectedAsset: asset, - project: newProject, - }); + const assetService = new AssetService(project); + const asset = await assetService.deleteTag(project.assets, tagName, selectedAsset); + if (asset) { + this.canvas.current.updateCanvasToolsRegions(asset); + this.setState({selectedAsset: asset}); + } } private onCtrlTagClicked = (tag: ITag): void => { diff --git a/src/services/assetService.ts b/src/services/assetService.ts index 99208ecdca..c87b3b795d 100644 --- a/src/services/assetService.ts +++ b/src/services/assetService.ts @@ -243,6 +243,9 @@ export class AssetService { tagName: string, transformer: (tags: string[]) => string[], currentAsset?: IAssetMetadata): Promise { + if (!assets) { + return; + } const assetKeys = Object.keys(assets); // Loop over assets and update if necessary for (const assetKey of assetKeys) { diff --git a/src/services/projectService.test.ts b/src/services/projectService.test.ts index 9f5b6278ed..f610e750a3 100644 --- a/src/services/projectService.test.ts +++ b/src/services/projectService.test.ts @@ -157,33 +157,4 @@ describe("Project Service", () => { await projectSerivce.delete(testProject); expect(storageProviderMock.deleteFile.mock.calls).toHaveLength(assets.length + 1); }); - - it("Deletes a tag from project", () => { - const tag = MockFactory.createTestTag(); - const project: IProject = { - ...MockFactory.createTestProject(), - tags: [tag], - }; - const projectService = new ProjectService(); - expect(projectSerivce.deleteTag(project, tag.name)).toEqual({ - ...project, - tags: [], - }); - }); - - it("Renames a tag within project", () => { - const tag = MockFactory.createTestTag(); - const project: IProject = { - ...MockFactory.createTestProject(), - tags: [tag], - }; - const projectService = new ProjectService(); - expect(projectSerivce.renameTag(project, tag.name, "test")).toEqual({ - ...project, - tags: [{ - name: "test", - color: expect.any(String), - }], - }); - }); }); diff --git a/src/services/projectService.ts b/src/services/projectService.ts index e092c5343d..3b868cf27f 100644 --- a/src/services/projectService.ts +++ b/src/services/projectService.ts @@ -1,14 +1,12 @@ import _ from "lodash"; import shortid from "shortid"; import { StorageProviderFactory } from "../providers/storage/storageProviderFactory"; -import { IProject, ISecurityToken, AppError, ErrorCode, AssetState, IAssetMetadata } from "../models/applicationState"; +import { IProject, ISecurityToken, AppError, ErrorCode, AssetState } from "../models/applicationState"; import Guard from "../common/guard"; import { constants } from "../common/constants"; import { ExportProviderFactory } from "../providers/export/exportProviderFactory"; import { decryptProject, encryptProject } from "../common/utils"; import packageJson from "../../package.json"; -import { AssetService } from "./assetService"; -import { forEachAsync } from "../common/extensions/array"; /** * Functions required for a project service @@ -20,8 +18,6 @@ export interface IProjectService { save(project: IProject, securityToken: ISecurityToken): Promise; delete(project: IProject): Promise; isDuplicate(project: IProject, projectList: IProject[]): boolean; - deleteTag(project: IProject, tagName: string): IProject; - renameTag(project: IProject, tagName: string, newTagName: string); } /** @@ -104,20 +100,6 @@ export default class ProjectService implements IProjectService { return (duplicateProjects !== undefined); } - public deleteTag(project: IProject, tagName: string): IProject { - return { - ...project, - tags: project.tags.filter((t) => t.name !== tagName), - }; - } - - public renameTag(project: IProject, tagName: string, newTagName: string): IProject { - return { - ...project, - tags: project.tags.map((t) => (t.name === tagName) ? {...t, name: newTagName} : t), - }; - } - private async saveExportSettings(project: IProject): Promise { if (!project.exportFormat || !project.exportFormat.providerType) { return Promise.resolve(); From f11edfafb5020832537240afb654bcbeb879d256 Mon Sep 17 00:00:00 2001 From: Tanner Barlow Date: Mon, 15 Apr 2019 12:50:13 -0700 Subject: [PATCH 06/16] Register mixins and run async loop --- .../pages/editorPage/editorPage.tsx | 7 +++--- src/services/assetService.test.ts | 5 ++++ src/services/assetService.ts | 24 ++----------------- 3 files changed, 10 insertions(+), 26 deletions(-) diff --git a/src/react/components/pages/editorPage/editorPage.tsx b/src/react/components/pages/editorPage/editorPage.tsx index bba44a2248..afbad529e7 100644 --- a/src/react/components/pages/editorPage/editorPage.tsx +++ b/src/react/components/pages/editorPage/editorPage.tsx @@ -316,7 +316,7 @@ export default class EditorPage extends React.Component (t.name === tagName) ? {...t, name: newTagName} : t), - } + }; this.setState({ project: newProject, selectedAsset: asset || selectedAsset, @@ -337,10 +337,9 @@ export default class EditorPage extends React.Component t.name !== tagName) - } + tags: project.tags.filter((t) => t.name !== tagName), + }; await this.props.actions.saveProject(newProject); - debugger; const assetService = new AssetService(project); const asset = await assetService.deleteTag(project.assets, tagName, selectedAsset); diff --git a/src/services/assetService.test.ts b/src/services/assetService.test.ts index ef84e11200..1bfce885d8 100644 --- a/src/services/assetService.test.ts +++ b/src/services/assetService.test.ts @@ -8,6 +8,7 @@ import { TFRecordsBuilder, FeatureType } from "../providers/export/tensorFlowRec import HtmlFileReader from "../common/htmlFileReader"; import { encodeFileURI } from "../common/utils"; import _ from "lodash"; +import registerMixins from "../registerMixins"; describe("Asset Service", () => { describe("Static Methods", () => { @@ -340,6 +341,10 @@ describe("Asset Service", () => { return project; } + beforeAll(() => { + registerMixins(); + }) + it("Deletes tag from assets", async () => { const tag1 = "tag1"; const tag2 = "tag2"; diff --git a/src/services/assetService.ts b/src/services/assetService.ts index c87b3b795d..8ec2fe5386 100644 --- a/src/services/assetService.ts +++ b/src/services/assetService.ts @@ -247,8 +247,9 @@ export class AssetService { return; } const assetKeys = Object.keys(assets); + // Loop over assets and update if necessary - for (const assetKey of assetKeys) { + await assetKeys.forEachAsync(async (assetKey) => { const asset = assets[assetKey]; if (asset.state !== AssetState.Tagged) { return; @@ -258,28 +259,7 @@ export class AssetService { if (updatedAssetMetadata) { await this.save(updatedAssetMetadata); } - } - if (currentAsset) { - return this.updateTagInAssetMetadata(currentAsset, tagName, transformer); - } - /* - TODO: Replace with async - - For some reason in tests, the `forEachAsync` is not recognized as a function - - await assetKeys.forEachAsync(async (assetKey) => { - const asset = project.assets[assetKey]; - if (asset.state !== AssetState.Tagged) { - return; - } - const assetMetadata = await assetService.getAssetMetadata(asset); - const updatedAssetMetadata = this.updateTagInAssetMetadata(assetMetadata, tagName, transformer); - if (updatedAssetMetadata) { - await assetService.save(updatedAssetMetadata); - } }); - - */ } /** From 57f430f023337f735985b5eff1381fc384aadf86 Mon Sep 17 00:00:00 2001 From: Tanner Barlow Date: Mon, 15 Apr 2019 13:41:11 -0700 Subject: [PATCH 07/16] Saving assets in async foreach loop --- .../pages/editorPage/editorPage.tsx | 79 +++++++++++++------ src/services/assetService.ts | 24 +++++- 2 files changed, 76 insertions(+), 27 deletions(-) diff --git a/src/react/components/pages/editorPage/editorPage.tsx b/src/react/components/pages/editorPage/editorPage.tsx index afbad529e7..5ebcbe5b86 100644 --- a/src/react/components/pages/editorPage/editorPage.tsx +++ b/src/react/components/pages/editorPage/editorPage.tsx @@ -229,7 +229,7 @@ export default class EditorPage extends React.Component
=> { - const { project, selectedAsset } = this.state; - const assetService = new AssetService(project); - const asset = await assetService.renameTag(project.assets, tagName, newTagName, selectedAsset); - - const newProject: IProject = { - ...project, - tags: project.tags.map((t) => (t.name === tagName) ? {...t, name: newTagName} : t), - }; + const { project } = this.state; this.setState({ - project: newProject, - selectedAsset: asset || selectedAsset, + project: { + ...project, + tags: project.tags.map((t) => (t.name === tagName) ? {...t, name: newTagName} : t), + } }, async () => { - await this.props.actions.saveProject(newProject); - if (asset) { - this.canvas.current.updateCanvasToolsRegions(asset); + await this.props.actions.saveProject(project); + if (this.canvas.current) { + this.canvas.current.updateCanvasToolsRegions(); } }); + + // const { project, selectedAsset } = this.state; + // const assetService = new AssetService(project); + // const asset = await assetService.renameTag(project.assets, tagName, newTagName, selectedAsset); + + // const newProject: IProject = { + // ...project, + // tags: project.tags.map((t) => (t.name === tagName) ? {...t, name: newTagName} : t), + // }; + // this.setState({ + // project: newProject, + // selectedAsset: asset || selectedAsset, + // }, async () => { + // await this.props.actions.saveProject(newProject); + // if (asset) { + // this.canvas.current.updateCanvasToolsRegions(asset); + // } + // }); } private confirmTagDeleted = (tagName: string): void => { @@ -333,20 +346,38 @@ export default class EditorPage extends React.Component => { - const { selectedAsset } = this.state; - const { project } = this.props; - const newProject: IProject = { + const { project, selectedAsset } = this.state; + const newProject = { ...project, tags: project.tags.filter((t) => t.name !== tagName), - }; - await this.props.actions.saveProject(newProject); + } - const assetService = new AssetService(project); + const assetService = new AssetService(newProject); const asset = await assetService.deleteTag(project.assets, tagName, selectedAsset); - if (asset) { - this.canvas.current.updateCanvasToolsRegions(asset); - this.setState({selectedAsset: asset}); - } + this.setState({ + project: newProject, + selectedAsset: asset || selectedAsset, + }, async () => { + await this.props.actions.saveProject(newProject); + if (this.canvas.current) { + this.canvas.current.updateCanvasToolsRegions(asset); + } + }); + + // const { selectedAsset } = this.state; + // const { project } = this.props; + // const newProject: IProject = { + // ...project, + // tags: project.tags.filter((t) => t.name !== tagName), + // }; + // await this.props.actions.saveProject(newProject); + + // const assetService = new AssetService(project); + // const asset = await assetService.deleteTag(project.assets, tagName, selectedAsset); + // if (asset) { + // this.canvas.current.updateCanvasToolsRegions(asset); + // this.setState({selectedAsset: asset}); + // } } private onCtrlTagClicked = (tag: ITag): void => { diff --git a/src/services/assetService.ts b/src/services/assetService.ts index 8ec2fe5386..53d05087f0 100644 --- a/src/services/assetService.ts +++ b/src/services/assetService.ts @@ -251,15 +251,33 @@ export class AssetService { // Loop over assets and update if necessary await assetKeys.forEachAsync(async (assetKey) => { const asset = assets[assetKey]; - if (asset.state !== AssetState.Tagged) { - return; - } const assetMetadata = await this.getAssetMetadata(asset); const updatedAssetMetadata = this.updateTagInAssetMetadata(assetMetadata, tagName, transformer); if (updatedAssetMetadata) { await this.save(updatedAssetMetadata); } }); + + // for (const assetKey of assetKeys) { + // const asset = assets[assetKey]; + // if (asset.state !== AssetState.Tagged) { + // debugger; + // return; + // } + // const assetMetadata = await this.getAssetMetadata(asset); + // debugger; + // const updatedAssetMetadata = this.updateTagInAssetMetadata(assetMetadata, tagName, transformer); + // debugger; + // if (updatedAssetMetadata) { + // await this.save(updatedAssetMetadata); + // } + // } + + if (currentAsset) { + const asset = this.updateTagInAssetMetadata(currentAsset, tagName, transformer); + debugger; + return asset; + } } /** From 33223630e94f117ebd5334d13e806a6a1fe4f3ec Mon Sep 17 00:00:00 2001 From: Tanner Barlow Date: Mon, 15 Apr 2019 13:58:37 -0700 Subject: [PATCH 08/16] Delete tag working --- src/services/assetService.ts | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/src/services/assetService.ts b/src/services/assetService.ts index 53d05087f0..1b31aeba4e 100644 --- a/src/services/assetService.ts +++ b/src/services/assetService.ts @@ -247,6 +247,7 @@ export class AssetService { return; } const assetKeys = Object.keys(assets); + let result: IAssetMetadata; // Loop over assets and update if necessary await assetKeys.forEachAsync(async (assetKey) => { @@ -255,29 +256,12 @@ export class AssetService { const updatedAssetMetadata = this.updateTagInAssetMetadata(assetMetadata, tagName, transformer); if (updatedAssetMetadata) { await this.save(updatedAssetMetadata); + if (currentAsset && currentAsset.asset.id === updatedAssetMetadata.asset.id) { + result = updatedAssetMetadata; + } } }); - - // for (const assetKey of assetKeys) { - // const asset = assets[assetKey]; - // if (asset.state !== AssetState.Tagged) { - // debugger; - // return; - // } - // const assetMetadata = await this.getAssetMetadata(asset); - // debugger; - // const updatedAssetMetadata = this.updateTagInAssetMetadata(assetMetadata, tagName, transformer); - // debugger; - // if (updatedAssetMetadata) { - // await this.save(updatedAssetMetadata); - // } - // } - - if (currentAsset) { - const asset = this.updateTagInAssetMetadata(currentAsset, tagName, transformer); - debugger; - return asset; - } + return result; } /** From d08b90bc874692e89e52691964277ca8ee1a09a0 Mon Sep 17 00:00:00 2001 From: Tanner Barlow Date: Mon, 15 Apr 2019 14:57:12 -0700 Subject: [PATCH 09/16] Rename tag function in editor page --- .../pages/editorPage/editorPage.tsx | 54 +++++-------------- src/services/assetService.test.ts | 2 +- 2 files changed, 14 insertions(+), 42 deletions(-) diff --git a/src/react/components/pages/editorPage/editorPage.tsx b/src/react/components/pages/editorPage/editorPage.tsx index 5ebcbe5b86..7c3aab68af 100644 --- a/src/react/components/pages/editorPage/editorPage.tsx +++ b/src/react/components/pages/editorPage/editorPage.tsx @@ -309,36 +309,23 @@ export default class EditorPage extends React.Component => { - const { project } = this.state; + const { project, selectedAsset } = this.state; + const newProject = { + ...project, + tags: project.tags.map((t) => (t.name === tagName) ? {...t, name: newTagName} : t), + }; + + const assetService = new AssetService(newProject); + const asset = await assetService.renameTag(project.assets, tagName, newTagName, selectedAsset); this.setState({ - project: { - ...project, - tags: project.tags.map((t) => (t.name === tagName) ? {...t, name: newTagName} : t), - } + project: newProject, + selectedAsset: asset || selectedAsset, }, async () => { - await this.props.actions.saveProject(project); + await this.props.actions.saveProject(newProject); if (this.canvas.current) { - this.canvas.current.updateCanvasToolsRegions(); + this.canvas.current.updateCanvasToolsRegions(asset); } }); - - // const { project, selectedAsset } = this.state; - // const assetService = new AssetService(project); - // const asset = await assetService.renameTag(project.assets, tagName, newTagName, selectedAsset); - - // const newProject: IProject = { - // ...project, - // tags: project.tags.map((t) => (t.name === tagName) ? {...t, name: newTagName} : t), - // }; - // this.setState({ - // project: newProject, - // selectedAsset: asset || selectedAsset, - // }, async () => { - // await this.props.actions.saveProject(newProject); - // if (asset) { - // this.canvas.current.updateCanvasToolsRegions(asset); - // } - // }); } private confirmTagDeleted = (tagName: string): void => { @@ -350,7 +337,7 @@ export default class EditorPage extends React.Component t.name !== tagName), - } + }; const assetService = new AssetService(newProject); const asset = await assetService.deleteTag(project.assets, tagName, selectedAsset); @@ -363,21 +350,6 @@ export default class EditorPage extends React.Component t.name !== tagName), - // }; - // await this.props.actions.saveProject(newProject); - - // const assetService = new AssetService(project); - // const asset = await assetService.deleteTag(project.assets, tagName, selectedAsset); - // if (asset) { - // this.canvas.current.updateCanvasToolsRegions(asset); - // this.setState({selectedAsset: asset}); - // } } private onCtrlTagClicked = (tag: ITag): void => { diff --git a/src/services/assetService.test.ts b/src/services/assetService.test.ts index 1bfce885d8..ee46e42447 100644 --- a/src/services/assetService.test.ts +++ b/src/services/assetService.test.ts @@ -343,7 +343,7 @@ describe("Asset Service", () => { beforeAll(() => { registerMixins(); - }) + }); it("Deletes tag from assets", async () => { const tag1 = "tag1"; From 1eb23e0a0f5889c8e7e1ab480415d16f806dfd2d Mon Sep 17 00:00:00 2001 From: Tanner Barlow Date: Tue, 16 Apr 2019 07:06:00 -0700 Subject: [PATCH 10/16] Fix tag removal test for editor page --- .../components/pages/editorPage/editorPage.test.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/react/components/pages/editorPage/editorPage.test.tsx b/src/react/components/pages/editorPage/editorPage.test.tsx index 786375aba5..4716f33bad 100644 --- a/src/react/components/pages/editorPage/editorPage.test.tsx +++ b/src/react/components/pages/editorPage/editorPage.test.tsx @@ -29,6 +29,7 @@ import { appInfo } from "../../../../common/appInfo"; import SplitPane from "react-split-pane"; import EditorSideBar from "./editorSideBar"; import Alert from "../../common/alert/alert"; +import registerMixins from "../../../../registerMixins"; function createComponent(store, props: IEditorPageProps): ReactWrapper { return mount( @@ -657,6 +658,11 @@ describe("Editor Page Component", () => { }); describe("Basic tag interaction tests", () => { + + beforeAll(() => { + registerMixins(); + }); + it("tags are initialized correctly", () => { const project = MockFactory.createTestProject(); const store = createReduxStore({ @@ -700,6 +706,9 @@ describe("Editor Page Component", () => { expect(getState(wrapper).project.tags).toEqual(project.tags); wrapper.find(".tag-content").last().simulate("click"); wrapper.find("i.tag-input-toolbar-icon.fas.fa-trash").simulate("click"); + wrapper.find("button.btn.btn-danger").simulate("click"); + + await MockFactory.flushUi(); const stateTags = getState(wrapper).project.tags; expect(stateTags).toHaveLength(project.tags.length - 1); From 48ac5db75c53356c3cd70ff2376587462bb8c79d Mon Sep 17 00:00:00 2001 From: Tanner Barlow Date: Tue, 16 Apr 2019 07:13:02 -0700 Subject: [PATCH 11/16] Clean up and docs --- src/react/components/pages/editorPage/editorPage.tsx | 9 +++++++++ src/services/assetService.test.ts | 3 +-- src/services/assetService.ts | 4 +++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/react/components/pages/editorPage/editorPage.tsx b/src/react/components/pages/editorPage/editorPage.tsx index 7c3aab68af..7cff737372 100644 --- a/src/react/components/pages/editorPage/editorPage.tsx +++ b/src/react/components/pages/editorPage/editorPage.tsx @@ -308,6 +308,11 @@ export default class EditorPage extends React.Component => { const { project, selectedAsset } = this.state; const newProject = { @@ -332,6 +337,10 @@ export default class EditorPage extends React.Component => { const { project, selectedAsset } = this.state; const newProject = { diff --git a/src/services/assetService.test.ts b/src/services/assetService.test.ts index ee46e42447..7e1350a441 100644 --- a/src/services/assetService.test.ts +++ b/src/services/assetService.test.ts @@ -369,7 +369,6 @@ describe("Asset Service", () => { }, ], ), - }; const project = populateProjectAssets(); @@ -422,8 +421,8 @@ describe("Asset Service", () => { }, ], ), - }; + const project = populateProjectAssets(); const assetService = new AssetService(project); await assetService.renameTag(project.assets, tag1, newTag, assetMetadata); diff --git a/src/services/assetService.ts b/src/services/assetService.ts index 1b31aeba4e..be40a968c6 100644 --- a/src/services/assetService.ts +++ b/src/services/assetService.ts @@ -234,9 +234,11 @@ export class AssetService { * Update tags within asset metadata files * @param assets The assets containing tags to update * @param tagName Name of tag to update within project + * @param transformer Function that accepts array of tags from a region and returns a modified array of tags * @param currentAsset Current asset being viewed. Makes changes and returns updated asset to avoid * needing to reload the asset in the editor page - * @param transformer Function that accepts array of tags from a region and returns a modified array of tags + * @returns Modified `currentAsset`. Returns `null` if asset did not need to be modified + * or if `currentAsset` is null or undefined */ private async updateAssetTags( assets: {[id: string]: IAsset}, From 390ab50f5ad121e2014002607169a57cc608efb1 Mon Sep 17 00:00:00 2001 From: Tanner Barlow Date: Tue, 16 Apr 2019 07:14:28 -0700 Subject: [PATCH 12/16] Lint fixes --- src/services/assetService.test.ts | 2 +- src/services/assetService.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/assetService.test.ts b/src/services/assetService.test.ts index 7e1350a441..02680fcd4d 100644 --- a/src/services/assetService.test.ts +++ b/src/services/assetService.test.ts @@ -422,7 +422,7 @@ describe("Asset Service", () => { ], ), }; - + const project = populateProjectAssets(); const assetService = new AssetService(project); await assetService.renameTag(project.assets, tag1, newTag, assetMetadata); diff --git a/src/services/assetService.ts b/src/services/assetService.ts index be40a968c6..4552624eae 100644 --- a/src/services/assetService.ts +++ b/src/services/assetService.ts @@ -237,7 +237,7 @@ export class AssetService { * @param transformer Function that accepts array of tags from a region and returns a modified array of tags * @param currentAsset Current asset being viewed. Makes changes and returns updated asset to avoid * needing to reload the asset in the editor page - * @returns Modified `currentAsset`. Returns `null` if asset did not need to be modified + * @returns Modified `currentAsset`. Returns `null` if asset did not need to be modified * or if `currentAsset` is null or undefined */ private async updateAssetTags( From f45cf244b21b1455be0ce86b2a007ed0ea81147c Mon Sep 17 00:00:00 2001 From: Tanner Barlow Date: Tue, 16 Apr 2019 07:25:56 -0700 Subject: [PATCH 13/16] Dummy commit to kick off build again --- src/react/components/pages/editorPage/editorPage.tsx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/react/components/pages/editorPage/editorPage.tsx b/src/react/components/pages/editorPage/editorPage.tsx index 7cff737372..24931b5ade 100644 --- a/src/react/components/pages/editorPage/editorPage.tsx +++ b/src/react/components/pages/editorPage/editorPage.tsx @@ -304,6 +304,9 @@ export default class EditorPage extends React.Component this.canvas.current.applyTag(tag.name)); } + /** + * Open confirm dialog for tag renaming + */ private confirmTagRenamed = (tagName: string, newTagName: string): void => { this.renameTagConfirm.current.open(tagName, newTagName); } @@ -333,6 +336,9 @@ export default class EditorPage extends React.Component { this.deleteTagConfirm.current.open(tagName); } From 466902fb2fc7a13c06428895a86bed81a38000df Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Fri, 19 Apr 2019 09:05:14 -0700 Subject: [PATCH 14/16] fix: Refactored project tag/delete updates --- src/common/mockFactory.ts | 16 ++-- .../components/common/tagInput/tagInput.tsx | 47 ++++++---- .../pages/editorPage/canvas.test.tsx | 2 +- .../components/pages/editorPage/canvas.tsx | 36 ++++---- .../pages/editorPage/editorPage.test.tsx | 18 ++-- .../pages/editorPage/editorPage.tsx | 88 ++++++++---------- src/redux/actions/actionTypes.ts | 2 + src/redux/actions/projectActions.ts | 89 +++++++++++++++++++ src/services/assetService.test.ts | 6 +- src/services/assetService.ts | 63 +++++-------- 10 files changed, 217 insertions(+), 150 deletions(-) diff --git a/src/common/mockFactory.ts b/src/common/mockFactory.ts index 1db2f08d61..8dfb8ef3e1 100644 --- a/src/common/mockFactory.ts +++ b/src/common/mockFactory.ts @@ -810,14 +810,16 @@ export default class MockFactory { */ public static projectActions(): IProjectActions { return { - loadProject: jest.fn((project: IProject) => Promise.resolve()), - saveProject: jest.fn((project: IProject) => Promise.resolve()), - deleteProject: jest.fn((project: IProject) => Promise.resolve()), + loadProject: jest.fn(() => Promise.resolve()), + saveProject: jest.fn(() => Promise.resolve()), + deleteProject: jest.fn(() => Promise.resolve()), closeProject: jest.fn(() => Promise.resolve()), - loadAssets: jest.fn((project: IProject) => Promise.resolve()), - exportProject: jest.fn((project: IProject) => Promise.resolve()), - loadAssetMetadata: jest.fn((project: IProject, asset: IAsset) => Promise.resolve()), - saveAssetMetadata: jest.fn((project: IProject, assetMetadata: IAssetMetadata) => Promise.resolve()), + loadAssets: jest.fn(() => Promise.resolve()), + exportProject: jest.fn(() => Promise.resolve()), + loadAssetMetadata: jest.fn(() => Promise.resolve()), + saveAssetMetadata: jest.fn(() => Promise.resolve()), + updateProjectTag: jest.fn(() => Promise.resolve()), + deleteProjectTag: jest.fn(() => Promise.resolve()), }; } diff --git a/src/react/components/common/tagInput/tagInput.tsx b/src/react/components/common/tagInput/tagInput.tsx index b1430c4ca0..56db6e8954 100644 --- a/src/react/components/common/tagInput/tagInput.tsx +++ b/src/react/components/common/tagInput/tagInput.tsx @@ -1,4 +1,4 @@ -import React, { KeyboardEvent } from "react"; +import React, { KeyboardEvent, RefObject } from "react"; import ReactDOM from "react-dom"; import Align from "rc-align"; import { randomIntInRange } from "../../../../common/utils"; @@ -10,6 +10,7 @@ import TagInputItem, { ITagInputItemProps, ITagClickProps } from "./tagInputItem import TagInputToolbar from "./tagInputToolbar"; import { toast } from "react-toastify"; import { strings } from "../../../../common/strings"; +import { string } from "prop-types"; // tslint:disable-next-line:no-var-requires const tagColors = require("../../common/tagColors.json"); @@ -72,7 +73,7 @@ export class TagInput extends React.Component { portalElement: defaultDOMNode(), }; - private tagItemRefs: { [id: string]: TagInputItem } = {}; + private tagItemRefs: Map> = new Map>(); private portalDiv = document.createElement("div"); public render() { @@ -109,7 +110,7 @@ export class TagInput extends React.Component { } {this.getColorPickerPortal()}
- {this.getTagItems()} + {this.renderTagItems()}
{ this.state.addTags && @@ -154,11 +155,13 @@ export class TagInput extends React.Component { } } - private getTagNode = (tag: ITag) => { + private getTagNode = (tag: ITag): Element => { if (!tag) { return defaultDOMNode(); } - return ReactDOM.findDOMNode(this.tagItemRefs[tag.name]) as Element; + + const itemRef = this.tagItemRefs.get(tag.name); + return (itemRef ? ReactDOM.findDOMNode(itemRef.current) : defaultDOMNode()) as Element; } private onEditTag = (tag: ITag) => { @@ -298,12 +301,15 @@ export class TagInput extends React.Component { return this.state.editingTagNode || document; } - private getTagItems = () => { - let props = this.getTagItemProps(); + private renderTagItems = () => { + let props = this.createTagItemProps(); const query = this.state.searchQuery; + this.tagItemRefs.clear(); + if (query.length) { props = props.filter((prop) => prop.tag.name.toLowerCase().includes(query.toLowerCase())); } + return props.map((prop) => { } private setTagItemRef = (item, tag) => { - if (item) { - this.tagItemRefs[tag.name] = item; - } + this.tagItemRefs.set(tag.name, item); + return item; } - private getTagItemProps = (): ITagInputItemProps[] => { + private createTagItemProps = (): ITagInputItemProps[] => { const tags = this.state.tags; const selectedRegionTagSet = this.getSelectedRegionTagSet(); - return tags.map((tag) => { - const item: ITagInputItemProps = { + + return tags.map((tag) => ( + { tag, index: tags.findIndex((t) => t.name === tag.name), isLocked: this.props.lockedTags && this.props.lockedTags.findIndex((t) => t === tag.name) > -1, @@ -331,9 +337,8 @@ export class TagInput extends React.Component { appliedToSelectedRegions: selectedRegionTagSet.has(tag.name), onClick: this.handleClick, onChange: this.updateTag, - }; - return item; - }); + } as ITagInputItemProps + )); } private getSelectedRegionTagSet = (): Set => { @@ -351,6 +356,7 @@ export class TagInput extends React.Component { private onAltClick = (tag: ITag, clickedColor: boolean) => { const { editingTag } = this.state; const newEditingTag = editingTag && editingTag.name === tag.name ? null : tag; + this.setState({ editingTag: newEditingTag, editingTagNode: this.getTagNode(newEditingTag), @@ -360,16 +366,16 @@ export class TagInput extends React.Component { } private handleClick = (tag: ITag, props: ITagClickProps) => { + // Lock tags if (props.ctrlKey && this.props.onCtrlTagClick) { this.props.onCtrlTagClick(tag); this.setState({ clickedColor: props.clickedColor }); - } else if (props.altKey) { + } else if (props.altKey) { // Edit tag this.onAltClick(tag, props.clickedColor); - } else { + } else { // Select tag const { editingTag, selectedTag } = this.state; const inEditMode = editingTag && tag.name === editingTag.name; const alreadySelected = selectedTag && selectedTag.name === tag.name; - const newEditingTag = inEditMode ? null : editingTag; this.setState({ @@ -398,12 +404,15 @@ export class TagInput extends React.Component { this.props.onTagDeleted(tag.name); return; } + const index = this.state.tags.indexOf(tag); const tags = this.state.tags.filter((t) => t.name !== tag.name); + this.setState({ tags, selectedTag: this.getNewSelectedTag(tags, index), }, () => this.props.onChange(tags)); + if (this.props.lockedTags.find((l) => l === tag.name)) { this.props.onLockedTagsChange( this.props.lockedTags.filter((lockedTag) => lockedTag !== tag.name), diff --git a/src/react/components/pages/editorPage/canvas.test.tsx b/src/react/components/pages/editorPage/canvas.test.tsx index d4ca52d109..a37c6f6a85 100644 --- a/src/react/components/pages/editorPage/canvas.test.tsx +++ b/src/react/components/pages/editorPage/canvas.test.tsx @@ -179,7 +179,7 @@ describe("Editor Canvas", () => { }); const canvas = wrapper.instance() as Canvas; expect(wrapper.state().currentAsset).toEqual(assetMetadata); - expect(() => canvas.updateCanvasToolsRegions()).not.toThrowError(); + expect(() => canvas.updateCanvasToolsRegionTags()).not.toThrowError(); }); it("canvas content source is updated when asset is deactivated", () => { diff --git a/src/react/components/pages/editorPage/canvas.tsx b/src/react/components/pages/editorPage/canvas.tsx index e73ce4efbd..0205e6e186 100644 --- a/src/react/components/pages/editorPage/canvas.tsx +++ b/src/react/components/pages/editorPage/canvas.tsx @@ -4,7 +4,7 @@ import { CanvasTools } from "vott-ct"; import { RegionData } from "vott-ct/lib/js/CanvasTools/Core/RegionData"; import { EditorMode, IAssetMetadata, - IProject, IRegion, RegionType, IBoundingBox, ISize, + IProject, IRegion, RegionType, } from "../../../../models/applicationState"; import CanvasHelpers from "./canvasHelpers"; import { AssetPreview, ContentSource } from "../../common/assetPreview/assetPreview"; @@ -73,7 +73,8 @@ export default class Canvas extends React.Component } public componentDidUpdate = async (prevProps: Readonly, prevState: Readonly) => { - if (this.props.selectedAsset.asset.id !== prevProps.selectedAsset.asset.id) { + // Handles asset changing + if (this.props.selectedAsset !== prevProps.selectedAsset) { this.setState({ currentAsset: this.props.selectedAsset }); } @@ -83,9 +84,16 @@ export default class Canvas extends React.Component this.editor.AS.setSelectionMode({ mode: this.props.selectionMode, template: options }); } + const assetIdChanged = this.state.currentAsset.asset.id !== prevState.currentAsset.asset.id; + + // When the selected asset has changed but is still the same asset id + if (!assetIdChanged && this.state.currentAsset !== prevState.currentAsset) { + this.refreshCanvasToolsRegions(); + } + // When the project tags change re-apply tags to regions if (this.props.project.tags !== prevProps.project.tags) { - this.updateCanvasToolsRegions(); + this.updateCanvasToolsRegionTags(); } // Handles when the canvas is enabled & disabled @@ -192,20 +200,12 @@ export default class Canvas extends React.Component return this.state.currentAsset.regions.filter((r) => selectedRegions.find((id) => r.id === id)); } - public updateCanvasToolsRegions = (asset?: IAssetMetadata): void => { - if (asset) { - this.setState({ - currentAsset: asset, - }); - this.clearAllRegions(); - this.addRegionsToCanvasTools(asset.regions); - } else { - for (const region of this.state.currentAsset.regions) { - this.editor.RM.updateTagsById( - region.id, - CanvasHelpers.getTagsDescriptor(this.props.project.tags, region), - ); - } + public updateCanvasToolsRegionTags = (): void => { + for (const region of this.state.currentAsset.regions) { + this.editor.RM.updateTagsById( + region.id, + CanvasHelpers.getTagsDescriptor(this.props.project.tags, region), + ); } } @@ -501,7 +501,7 @@ export default class Canvas extends React.Component this.editor.RM.updateTagsById(update.id, CanvasHelpers.getTagsDescriptor(this.props.project.tags, update)); } this.updateAssetRegions(updatedRegions); - this.updateCanvasToolsRegions(); + this.updateCanvasToolsRegionTags(); } /** diff --git a/src/react/components/pages/editorPage/editorPage.test.tsx b/src/react/components/pages/editorPage/editorPage.test.tsx index 4716f33bad..195a40e52d 100644 --- a/src/react/components/pages/editorPage/editorPage.test.tsx +++ b/src/react/components/pages/editorPage/editorPage.test.tsx @@ -126,12 +126,10 @@ describe("Editor Page Component", () => { const wrapper = createComponent(store, props); const editorPage = wrapper.find(EditorPage).childAt(0); - expect(getState(wrapper).project).toBeNull(); editorPage.props().project = testProject; await MockFactory.flushUi(); expect(editorPage.props().project).toEqual(testProject); - expect(getState(wrapper).project).toEqual(testProject); }); it("Loads and merges project assets with asset provider assets when state changes", async () => { @@ -671,7 +669,7 @@ describe("Editor Page Component", () => { }); const wrapper = createComponent(store, MockFactory.editorPageProps()); - expect(getState(wrapper).project.tags).toEqual(project.tags); + expect(wrapper.props().project.tags).toEqual(project.tags); }); it("create a new tag from text box", () => { @@ -681,16 +679,16 @@ describe("Editor Page Component", () => { currentProject: project, }); const wrapper = createComponent(store, MockFactory.editorPageProps()); - expect(getState(wrapper).project.tags).toEqual(project.tags); + expect(wrapper.props().project.tags).toEqual(project.tags); const newTagName = "My new tag"; wrapper.find("div.tag-input-toolbar-item.plus").simulate("click"); wrapper.find(".tag-input-box").simulate("keydown", { key: "Enter", target: { value: newTagName } }); - const stateTags = getState(wrapper).project.tags; + const projectTags = wrapper.props().project.tags; - expect(stateTags).toHaveLength(project.tags.length + 1); - expect(stateTags[stateTags.length - 1].name).toEqual(newTagName); + expect(projectTags).toHaveLength(project.tags.length + 1); + expect(projectTags[projectTags.length - 1].name).toEqual(newTagName); }); it("Remove a tag", async () => { @@ -703,15 +701,15 @@ describe("Editor Page Component", () => { const wrapper = createComponent(store, MockFactory.editorPageProps()); await waitForSelectedAsset(wrapper); - expect(getState(wrapper).project.tags).toEqual(project.tags); + expect(wrapper.props().project.tags).toEqual(project.tags); wrapper.find(".tag-content").last().simulate("click"); wrapper.find("i.tag-input-toolbar-icon.fas.fa-trash").simulate("click"); wrapper.find("button.btn.btn-danger").simulate("click"); await MockFactory.flushUi(); - const stateTags = getState(wrapper).project.tags; - expect(stateTags).toHaveLength(project.tags.length - 1); + const projectTags = wrapper.props().project.tags; + expect(projectTags).toHaveLength(project.tags.length - 1); }); it("Adds tag to locked tags when CmdOrCtrl clicked", async () => { diff --git a/src/react/components/pages/editorPage/editorPage.tsx b/src/react/components/pages/editorPage/editorPage.tsx index 24931b5ade..fc221d2e1a 100644 --- a/src/react/components/pages/editorPage/editorPage.tsx +++ b/src/react/components/pages/editorPage/editorPage.tsx @@ -29,7 +29,6 @@ import EditorSideBar from "./editorSideBar"; import { EditorToolbar } from "./editorToolbar"; import Alert from "../../common/alert/alert"; import Confirm from "../../common/confirm/confirm"; -import ProjectService from "../../../../services/projectService"; // tslint:disable-next-line:no-var-requires const tagColors = require("../../common/tagColors.json"); @@ -52,8 +51,6 @@ export interface IEditorPageProps extends RouteComponentProps, React.Props { public state: IEditorPageState = { - project: this.props.project, selectedTag: null, lockedTags: [], selectionMode: SelectionMode.RECT, @@ -135,7 +131,7 @@ export default class EditorPage extends React.Component) { if (this.props.project && this.state.assets.length === 0) { await this.loadProjectAssets(); } @@ -143,10 +139,13 @@ export default class EditorPage extends React.Component
+ onConfirm={this.onTagRenamed} /> + onConfirm={this.onTagDeleted} />
=> { - const { project, selectedAsset } = this.state; - const newProject = { - ...project, - tags: project.tags.map((t) => (t.name === tagName) ? {...t, name: newTagName} : t), - }; + const assetUpdates = await this.props.actions.updateProjectTag(this.props.project, tagName, newTagName); + const selectedAsset = assetUpdates.find((am) => am.asset.id === this.state.selectedAsset.asset.id); - const assetService = new AssetService(newProject); - const asset = await assetService.renameTag(project.assets, tagName, newTagName, selectedAsset); - this.setState({ - project: newProject, - selectedAsset: asset || selectedAsset, - }, async () => { - await this.props.actions.saveProject(newProject); - if (this.canvas.current) { - this.canvas.current.updateCanvasToolsRegions(asset); + if (selectedAsset) { + if (selectedAsset) { + this.setState({ selectedAsset }); } - }); + } } /** @@ -348,23 +338,12 @@ export default class EditorPage extends React.Component => { - const { project, selectedAsset } = this.state; - const newProject = { - ...project, - tags: project.tags.filter((t) => t.name !== tagName), - }; + const assetUpdates = await this.props.actions.deleteProjectTag(this.props.project, tagName); + const selectedAsset = assetUpdates.find((am) => am.asset.id === this.state.selectedAsset.asset.id); - const assetService = new AssetService(newProject); - const asset = await assetService.deleteTag(project.assets, tagName, selectedAsset); - this.setState({ - project: newProject, - selectedAsset: asset || selectedAsset, - }, async () => { - await this.props.actions.saveProject(newProject); - if (this.canvas.current) { - this.canvas.current.updateCanvasToolsRegions(asset); - } - }); + if (selectedAsset) { + this.setState({ selectedAsset }); + } } private onCtrlTagClicked = (tag: ITag): void => { @@ -502,17 +481,13 @@ export default class EditorPage extends React.Component { + private onTagsChanged = async (tags) => { const project = { ...this.props.project, tags, }; - this.setState({ project }, async () => { - await this.props.actions.saveProject(project); - if (this.canvas.current) { - this.canvas.current.updateCanvasToolsRegions(); - } - }); + + await this.props.actions.saveProject(project); } private onLockedTagsChanged = (lockedTags: string[]) => { @@ -677,4 +652,19 @@ export default class EditorPage extends React.Component { + const updatedAssets = [...this.state.assets]; + updatedAssets.forEach((asset) => { + const projectAsset = this.props.project.assets[asset.id]; + if (projectAsset) { + asset.state = projectAsset.state; + } + }); + + this.setState({ assets: updatedAssets }); + } } diff --git a/src/redux/actions/actionTypes.ts b/src/redux/actions/actionTypes.ts index 6f1ad9623c..2116460825 100644 --- a/src/redux/actions/actionTypes.ts +++ b/src/redux/actions/actionTypes.ts @@ -16,6 +16,8 @@ export enum ActionTypes { CLOSE_PROJECT_SUCCESS = "CLOSE_PROJECT_SUCCESS", LOAD_PROJECT_ASSETS_SUCCESS = "LOAD_PROJECT_ASSETS_SUCCESS", EXPORT_PROJECT_SUCCESS = "EXPORT_PROJECT_SUCCESS", + UPDATE_PROJECT_TAG_SUCCESS = "UPDATE_PROJECT_TAG_SUCCESS", + DELETE_PROJECT_TAG_SUCCESS = "DELETE_PROJECT_TAG_SUCCESS", // Connections LOAD_CONNECTION_SUCCESS = "LOAD_CONNECTION_SUCCESS", diff --git a/src/redux/actions/projectActions.ts b/src/redux/actions/projectActions.ts index 0b9b1b3670..5d853ec6ad 100644 --- a/src/redux/actions/projectActions.ts +++ b/src/redux/actions/projectActions.ts @@ -30,6 +30,8 @@ export default interface IProjectActions { loadAssets(project: IProject): Promise; loadAssetMetadata(project: IProject, asset: IAsset): Promise; saveAssetMetadata(project: IProject, assetMetadata: IAssetMetadata): Promise; + updateProjectTag(project: IProject, oldTagName: string, newTagName: string): Promise; + deleteProjectTag(project: IProject, tagName): Promise; } /** @@ -187,6 +189,69 @@ export function saveAssetMetadata( }; } +/** + * Updates a project and all asset references from oldTagName to newTagName + * @param project The project to update tags + * @param oldTagName The old tag name + * @param newTagName The new tag name + */ +export function updateProjectTag(project: IProject, oldTagName: string, newTagName: string) + : (dispatch: Dispatch, getState: () => IApplicationState) => Promise { + return async (dispatch: Dispatch, getState: () => IApplicationState) => { + // Find tags to rename + const assetService = new AssetService(project); + const assetUpdates = await assetService.renameTag(oldTagName, newTagName); + + // Save updated assets + await assetUpdates.forEachAsync(async (assetMetadata) => { + await saveAssetMetadata(project, assetMetadata)(dispatch); + }); + + const currentProject = getState().currentProject; + const updatedProject = { + ...currentProject, + tags: project.tags.map((t) => (t.name === oldTagName) ? { ...t, name: newTagName } : t), + }; + + // Save updated project tags + await saveProject(updatedProject)(dispatch, getState); + dispatch(updateProjectTagAction(updatedProject)); + + return assetUpdates; + }; +} + +/** + * Updates a project and all asset references from oldTagName to newTagName + * @param project The project to delete tags + * @param tagName The tag to delete + */ +export function deleteProjectTag(project: IProject, tagName) + : (dispatch: Dispatch, getState: () => IApplicationState) => Promise { + return async (dispatch: Dispatch, getState: () => IApplicationState) => { + // Find tags to rename + const assetService = new AssetService(project); + const assetUpdates = await assetService.deleteTag(tagName); + + // Save updated assets + await assetUpdates.forEachAsync(async (assetMetadata) => { + await saveAssetMetadata(project, assetMetadata)(dispatch); + }); + + const currentProject = getState().currentProject; + const updatedProject = { + ...currentProject, + tags: project.tags.filter((t) => t.name !== tagName), + }; + + // Save updated project tags + await saveProject(updatedProject)(dispatch, getState); + dispatch(deleteProjectTagAction(updatedProject)); + + return assetUpdates; + }; +} + /** * Initialize export provider, get export data and dispatch export project action * @param project - Project to export @@ -267,6 +332,20 @@ export interface IExportProjectAction extends IPayloadAction { type: ActionTypes.EXPORT_PROJECT_SUCCESS; } +/** + * Update Project Tag action type + */ +export interface IUpdateProjectTagAction extends IPayloadAction { + type: ActionTypes.UPDATE_PROJECT_TAG_SUCCESS; +} + +/** + * Delete project tag action type + */ +export interface IDeleteProjectTagAction extends IPayloadAction { + type: ActionTypes.DELETE_PROJECT_TAG_SUCCESS; +} + /** * Instance of Load Project action */ @@ -303,3 +382,13 @@ export const saveAssetMetadataAction = */ export const exportProjectAction = createPayloadAction(ActionTypes.EXPORT_PROJECT_SUCCESS); +/** + * Instance of Update project tag action + */ +export const updateProjectTagAction = + createPayloadAction(ActionTypes.UPDATE_PROJECT_TAG_SUCCESS); +/** + * Instance of Delete project tag action + */ +export const deleteProjectTagAction = + createPayloadAction(ActionTypes.DELETE_PROJECT_TAG_SUCCESS); diff --git a/src/services/assetService.test.ts b/src/services/assetService.test.ts index 02680fcd4d..e9037908c2 100644 --- a/src/services/assetService.test.ts +++ b/src/services/assetService.test.ts @@ -373,7 +373,7 @@ describe("Asset Service", () => { const project = populateProjectAssets(); const assetService = new AssetService(project); - await assetService.deleteTag(project.assets, tag1, assetMetadata); + await assetService.deleteTag(tag1); expect(saveMetadata).toBeCalledWith(expectedAssetMetadata); }); @@ -393,7 +393,7 @@ describe("Asset Service", () => { const expectedAssetMetadata: IAssetMetadata = MockFactory.createTestAssetMetadata(asset, []); const project = populateProjectAssets(); const assetService = new AssetService(project); - await assetService.deleteTag(project.assets, tag1, assetMetadata); + await assetService.deleteTag(tag1); expect(saveMetadata).toBeCalledWith(expectedAssetMetadata); }); @@ -425,7 +425,7 @@ describe("Asset Service", () => { const project = populateProjectAssets(); const assetService = new AssetService(project); - await assetService.renameTag(project.assets, tag1, newTag, assetMetadata); + await assetService.renameTag(tag1, newTag); expect(saveMetadata).toBeCalledWith(expectedAssetMetadata); }); }); diff --git a/src/services/assetService.ts b/src/services/assetService.ts index 4552624eae..2ab0ffabd0 100644 --- a/src/services/assetService.ts +++ b/src/services/assetService.ts @@ -206,64 +206,38 @@ export class AssetService { /** * Delete a tag from asset metadata files - * @param assets The assets containing tag to delete * @param tagName Name of tag to delete - * @param currentAsset Current asset being viewed. Makes changes and returns updated asset to avoid - * needing to reload the asset in the editor page */ - public async deleteTag(assets: {[id: string]: IAsset}, tagName: string, - currentAsset?: IAssetMetadata): Promise { + public async deleteTag(tagName: string): Promise { const transformer = (tags) => tags.filter((t) => t !== tagName); - return await this.updateAssetTags(assets, tagName, transformer, currentAsset); + return await this.getUpdatedAssets(tagName, transformer); } /** * Rename a tag within asset metadata files - * @param assets The assets containing tag to rename * @param tagName Name of tag to rename - * @param currentAsset Current asset being viewed. Makes changes and returns updated asset to avoid - * needing to reload the asset in the editor page */ - public async renameTag(assets: {[id: string]: IAsset}, tagName: string, newTagName: string, - currentAsset?: IAssetMetadata): Promise { + public async renameTag(tagName: string, newTagName: string): Promise { const transformer = (tags) => tags.map((t) => (t === tagName) ? newTagName : t); - return await this.updateAssetTags(assets, tagName, transformer, currentAsset); + return await this.getUpdatedAssets(tagName, transformer); } /** * Update tags within asset metadata files - * @param assets The assets containing tags to update * @param tagName Name of tag to update within project * @param transformer Function that accepts array of tags from a region and returns a modified array of tags - * @param currentAsset Current asset being viewed. Makes changes and returns updated asset to avoid - * needing to reload the asset in the editor page - * @returns Modified `currentAsset`. Returns `null` if asset did not need to be modified - * or if `currentAsset` is null or undefined */ - private async updateAssetTags( - assets: {[id: string]: IAsset}, - tagName: string, - transformer: (tags: string[]) => string[], - currentAsset?: IAssetMetadata): Promise { - if (!assets) { - return; - } - const assetKeys = Object.keys(assets); - let result: IAssetMetadata; - + private async getUpdatedAssets(tagName: string, transformer: (tags: string[]) => string[]) + : Promise { // Loop over assets and update if necessary - await assetKeys.forEachAsync(async (assetKey) => { - const asset = assets[assetKey]; + const updates = await _.values(this.project.assets).mapAsync(async (asset) => { const assetMetadata = await this.getAssetMetadata(asset); - const updatedAssetMetadata = this.updateTagInAssetMetadata(assetMetadata, tagName, transformer); - if (updatedAssetMetadata) { - await this.save(updatedAssetMetadata); - if (currentAsset && currentAsset.asset.id === updatedAssetMetadata.asset.id) { - result = updatedAssetMetadata; - } - } + const isUpdated = this.updateTagInAssetMetadata(assetMetadata, tagName, transformer); + + return isUpdated ? assetMetadata : null; }); - return result; + + return updates.filter((assetMetadata) => !!assetMetadata); } /** @@ -273,9 +247,12 @@ export class AssetService { * @param transformer Function that accepts array of tags from a region and returns a modified array of tags * @returns Modified asset metadata object or null if object does not need to be modified */ - private updateTagInAssetMetadata(assetMetadata: IAssetMetadata, tagName: string, - transformer: (tags: string[]) => string[]): IAssetMetadata { + private updateTagInAssetMetadata( + assetMetadata: IAssetMetadata, + tagName: string, + transformer: (tags: string[]) => string[]): boolean { let foundTag = false; + for (const region of assetMetadata.regions) { if (region.tags.find((t) => t === tagName)) { foundTag = true; @@ -285,15 +262,15 @@ export class AssetService { if (foundTag) { assetMetadata.regions = assetMetadata.regions.filter((region) => region.tags.length > 0); assetMetadata.asset.state = (assetMetadata.regions.length) ? AssetState.Tagged : AssetState.Visited; - return assetMetadata; + return true; } - return null; + + return false; } private async getRegionsFromTFRecord(asset: IAsset): Promise { const objectArray = await this.getTFRecordMetadata(asset); const regions: IRegion[] = []; - const tags: string[] = []; // Add Regions from TFRecord in Regions for (let index = 0; index < objectArray.textArray.length; index++) { From d1cd728454e26e015da15d276f1db4d548654566 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Fri, 19 Apr 2019 09:59:29 -0700 Subject: [PATCH 15/16] test: Refactored editor page tests --- .../pages/editorPage/editorPage.test.tsx | 42 ++++++++++++------- .../pages/editorPage/editorPage.tsx | 8 ++-- src/services/assetService.test.ts | 27 ++++++------ 3 files changed, 44 insertions(+), 33 deletions(-) diff --git a/src/react/components/pages/editorPage/editorPage.test.tsx b/src/react/components/pages/editorPage/editorPage.test.tsx index 195a40e52d..f8a717366f 100644 --- a/src/react/components/pages/editorPage/editorPage.test.tsx +++ b/src/react/components/pages/editorPage/editorPage.test.tsx @@ -30,6 +30,7 @@ import SplitPane from "react-split-pane"; import EditorSideBar from "./editorSideBar"; import Alert from "../../common/alert/alert"; import registerMixins from "../../../../registerMixins"; +import { TagInput } from "../../common/tagInput/tagInput"; function createComponent(store, props: IEditorPageProps): ReactWrapper { return mount( @@ -162,7 +163,7 @@ describe("Editor Page Component", () => { }); }); - it("Raises onAssetSelected handler when an asset is selected from the sidebar", async () => { + it("Default asset is loaded and saved during initial page rendering", async () => { // create test project and asset const testProject = MockFactory.createTestProject("TestProject"); const defaultAsset = testAssets[0]; @@ -180,6 +181,7 @@ describe("Editor Page Component", () => { const editorPage = wrapper.find(EditorPage).childAt(0) as ReactWrapper; await MockFactory.flushUi(); + wrapper.update(); const expectedAsset = editorPage.state().assets[0]; const partialProject = { @@ -669,26 +671,31 @@ describe("Editor Page Component", () => { }); const wrapper = createComponent(store, MockFactory.editorPageProps()); - expect(wrapper.props().project.tags).toEqual(project.tags); + expect(wrapper.find(TagInput).props().tags).toEqual(project.tags); }); - it("create a new tag from text box", () => { + it("create a new tag updates project tags", async () => { const project = MockFactory.createTestProject(); const store = createReduxStore({ ...MockFactory.initialState(), currentProject: project, }); + const wrapper = createComponent(store, MockFactory.editorPageProps()); - expect(wrapper.props().project.tags).toEqual(project.tags); + await waitForSelectedAsset(wrapper); - const newTagName = "My new tag"; - wrapper.find("div.tag-input-toolbar-item.plus").simulate("click"); - wrapper.find(".tag-input-box").simulate("keydown", { key: "Enter", target: { value: newTagName } }); + const newTag = MockFactory.createTestTag("NewTag"); + const updatedTags = [...project.tags, newTag]; + wrapper.find(TagInput).props().onChange(updatedTags); + + await MockFactory.flushUi(); + wrapper.update(); - const projectTags = wrapper.props().project.tags; + const editorPage = wrapper.find(EditorPage).childAt(0) as ReactWrapper; + const projectTags = editorPage.props().project.tags; - expect(projectTags).toHaveLength(project.tags.length + 1); - expect(projectTags[projectTags.length - 1].name).toEqual(newTagName); + expect(projectTags).toHaveLength(updatedTags.length); + expect(projectTags[projectTags.length - 1].name).toEqual(newTag.name); }); it("Remove a tag", async () => { @@ -701,14 +708,19 @@ describe("Editor Page Component", () => { const wrapper = createComponent(store, MockFactory.editorPageProps()); await waitForSelectedAsset(wrapper); - expect(wrapper.props().project.tags).toEqual(project.tags); - wrapper.find(".tag-content").last().simulate("click"); - wrapper.find("i.tag-input-toolbar-icon.fas.fa-trash").simulate("click"); - wrapper.find("button.btn.btn-danger").simulate("click"); + const tagToDelete = project.tags[project.tags.length - 1]; + wrapper.find(TagInput).props().onTagDeleted(tagToDelete.name); + + // Accept the modal delete warning + wrapper.update(); + wrapper.find(".modal-footer button").first().simulate("click"); await MockFactory.flushUi(); + wrapper.update(); + + const editorPage = wrapper.find(EditorPage).childAt(0) as ReactWrapper; + const projectTags = editorPage.props().project.tags; - const projectTags = wrapper.props().project.tags; expect(projectTags).toHaveLength(project.tags.length - 1); }); diff --git a/src/react/components/pages/editorPage/editorPage.tsx b/src/react/components/pages/editorPage/editorPage.tsx index fc221d2e1a..c4d3a3b723 100644 --- a/src/react/components/pages/editorPage/editorPage.tsx +++ b/src/react/components/pages/editorPage/editorPage.tsx @@ -139,15 +139,17 @@ export default class EditorPage extends React.Component { const assetMetadata = MockFactory.createTestAssetMetadata(asset, [region]); AssetService.prototype.getAssetMetadata = jest.fn((asset: IAsset) => Promise.resolve(assetMetadata)); - const saveMetadata = jest.fn(); - AssetService.prototype.save = saveMetadata; - const expectedAssetMetadata: IAssetMetadata = { ...MockFactory.createTestAssetMetadata( asset, @@ -373,8 +370,10 @@ describe("Asset Service", () => { const project = populateProjectAssets(); const assetService = new AssetService(project); - await assetService.deleteTag(tag1); - expect(saveMetadata).toBeCalledWith(expectedAssetMetadata); + const assetUpdates = await assetService.deleteTag(tag1); + + expect(assetUpdates).toHaveLength(1); + expect(assetUpdates[0]).toEqual(expectedAssetMetadata); }); it("Deletes empty regions after deleting only tag from region", async () => { @@ -387,14 +386,13 @@ describe("Asset Service", () => { const assetMetadata = MockFactory.createTestAssetMetadata(asset, [region]); AssetService.prototype.getAssetMetadata = jest.fn((asset: IAsset) => Promise.resolve(assetMetadata)); - const saveMetadata = jest.fn(); - AssetService.prototype.save = saveMetadata; - const expectedAssetMetadata: IAssetMetadata = MockFactory.createTestAssetMetadata(asset, []); const project = populateProjectAssets(); const assetService = new AssetService(project); - await assetService.deleteTag(tag1); - expect(saveMetadata).toBeCalledWith(expectedAssetMetadata); + const assetUpdates = await assetService.deleteTag(tag1); + + expect(assetUpdates).toHaveLength(1); + expect(assetUpdates[0]).toEqual(expectedAssetMetadata); }); it("Updates renamed tag within all assets", async () => { @@ -408,9 +406,6 @@ describe("Asset Service", () => { const assetMetadata = MockFactory.createTestAssetMetadata(asset, [region]); AssetService.prototype.getAssetMetadata = jest.fn((asset: IAsset) => Promise.resolve(assetMetadata)); - const saveMetadata = jest.fn(); - AssetService.prototype.save = saveMetadata; - const expectedAssetMetadata: IAssetMetadata = { ...MockFactory.createTestAssetMetadata( asset, @@ -425,8 +420,10 @@ describe("Asset Service", () => { const project = populateProjectAssets(); const assetService = new AssetService(project); - await assetService.renameTag(tag1, newTag); - expect(saveMetadata).toBeCalledWith(expectedAssetMetadata); + const assetUpdates = await assetService.renameTag(tag1, newTag); + + expect(assetUpdates).toHaveLength(1); + expect(assetUpdates[0]).toEqual(expectedAssetMetadata); }); }); }); From 4d99c1cdfbda85e2a1fa67e6a85e0aa2c47e82c8 Mon Sep 17 00:00:00 2001 From: Wallace Breza Date: Fri, 19 Apr 2019 11:57:13 -0700 Subject: [PATCH 16/16] test: Verify tag update/delete project actions --- .../components/common/tagInput/tagInput.tsx | 1 - src/redux/actions/projectActions.test.ts | 85 ++++++++++++++++++- 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/src/react/components/common/tagInput/tagInput.tsx b/src/react/components/common/tagInput/tagInput.tsx index 56db6e8954..540b540171 100644 --- a/src/react/components/common/tagInput/tagInput.tsx +++ b/src/react/components/common/tagInput/tagInput.tsx @@ -10,7 +10,6 @@ import TagInputItem, { ITagInputItemProps, ITagClickProps } from "./tagInputItem import TagInputToolbar from "./tagInputToolbar"; import { toast } from "react-toastify"; import { strings } from "../../../../common/strings"; -import { string } from "prop-types"; // tslint:disable-next-line:no-var-requires const tagColors = require("../../common/tagColors.json"); diff --git a/src/redux/actions/projectActions.test.ts b/src/redux/actions/projectActions.test.ts index 8a8197ed37..f9a49d5827 100644 --- a/src/redux/actions/projectActions.test.ts +++ b/src/redux/actions/projectActions.test.ts @@ -1,3 +1,4 @@ +import _ from "lodash"; import createMockStore, { MockStoreEnhanced } from "redux-mock-store"; import { ActionTypes } from "./actionTypes"; import * as projectActions from "./projectActions"; @@ -11,24 +12,29 @@ jest.mock("../../services/assetService"); import { AssetService } from "../../services/assetService"; import { ExportProviderFactory } from "../../providers/export/exportProviderFactory"; import { ExportAssetState, IExportProvider } from "../../providers/export/exportProvider"; -import { IApplicationState } from "../../models/applicationState"; +import { IApplicationState, IProject } from "../../models/applicationState"; import initialState from "../store/initialState"; import { appInfo } from "../../common/appInfo"; +import registerMixins from "../../registerMixins"; describe("Project Redux Actions", () => { let store: MockStoreEnhanced; let projectServiceMock: jest.Mocked; const appSettings = MockFactory.appSettings(); + beforeAll(registerMixins); + beforeEach(() => { const middleware = [thunk]; const mockState: IApplicationState = { ...initialState, appSettings, }; + store = createMockStore(middleware)(mockState); projectServiceMock = ProjectService as jest.Mocked; projectServiceMock.prototype.load = jest.fn((project) => Promise.resolve(project)); + projectServiceMock.prototype.save = jest.fn((project) => Promise.resolve(project)); }); it("Load Project action resolves a promise and dispatches redux action", async () => { @@ -255,4 +261,81 @@ describe("Project Redux Actions", () => { expect(mockExportProvider.export).toHaveBeenCalled(); }); + + describe("Updating project tags", () => { + let project: IProject = null; + + beforeEach(() => { + project = MockFactory.createTestProject("TestProject"); + const middleware = [thunk]; + const mockState: IApplicationState = { + ...initialState, + currentProject: project, + appSettings, + }; + + store = createMockStore(middleware)(mockState); + }); + + it("Updates tags across all project assets when a tag is renamed", async () => { + const projectAssets = _.values(project.assets); + const updatedTag = project.tags[project.tags.length - 1]; + + const updatedAssets = [ + MockFactory.createTestAssetMetadata(projectAssets[0]), + MockFactory.createTestAssetMetadata(projectAssets[1]), + ]; + + const expectedTagName = `${updatedTag.name} - updated`; + + const assetServiceMock = AssetService as jest.Mocked; + assetServiceMock.prototype.renameTag = jest.fn(() => Promise.resolve(updatedAssets)); + + const actualUpdatedAssets = await projectActions.updateProjectTag( + project, + updatedTag.name, + expectedTagName, + )(store.dispatch, store.getState); + + const actions = store.getActions(); + + expect(actions.length).toEqual(5); + expect(actions[0].type).toEqual(ActionTypes.SAVE_ASSET_METADATA_SUCCESS); + expect(actions[1].type).toEqual(ActionTypes.SAVE_ASSET_METADATA_SUCCESS); + expect(actions[2].type).toEqual(ActionTypes.SAVE_PROJECT_SUCCESS); + expect(actions[3].type).toEqual(ActionTypes.LOAD_PROJECT_SUCCESS); + expect(actions[4].type).toEqual(ActionTypes.UPDATE_PROJECT_TAG_SUCCESS); + + expect(actualUpdatedAssets).toEqual(updatedAssets); + }); + + it("Deletes tags across all project assets when a tag is renamed", async () => { + const projectAssets = _.values(project.assets); + const deletedTag = project.tags[project.tags.length - 1]; + + const updatedAssets = [ + MockFactory.createTestAssetMetadata(projectAssets[0]), + MockFactory.createTestAssetMetadata(projectAssets[1]), + ]; + + const assetServiceMock = AssetService as jest.Mocked; + assetServiceMock.prototype.deleteTag = jest.fn(() => Promise.resolve(updatedAssets)); + + const actualUpdatedAssets = await projectActions.deleteProjectTag( + project, + deletedTag.name, + )(store.dispatch, store.getState); + + const actions = store.getActions(); + + expect(actions.length).toEqual(5); + expect(actions[0].type).toEqual(ActionTypes.SAVE_ASSET_METADATA_SUCCESS); + expect(actions[1].type).toEqual(ActionTypes.SAVE_ASSET_METADATA_SUCCESS); + expect(actions[2].type).toEqual(ActionTypes.SAVE_PROJECT_SUCCESS); + expect(actions[3].type).toEqual(ActionTypes.LOAD_PROJECT_SUCCESS); + expect(actions[4].type).toEqual(ActionTypes.DELETE_PROJECT_TAG_SUCCESS); + + expect(actualUpdatedAssets).toEqual(updatedAssets); + }); + }); });