Update project when in-use connection changes AB#16921 (#564)

* Update current project when connection changes

* Removed redundant test in editor page

* Updated toolbar to remove duplicate logic

* Addressed PR feedback

src/common/mockFactory.ts

@@ -2,7 +2,7 @@ import shortid from "shortid";
 import {
     AssetState, AssetType, IApplicationState, IAppSettings, IAsset, IAssetMetadata,
     IConnection, IExportFormat, IProject, ITag, StorageType, ISecurityToken,
-    EditorMode, IAppError, IProjectVideoSettings, AppError, ErrorCode,
+    EditorMode, IAppError, IProjectVideoSettings, ErrorCode,
     IPoint, IRegion, RegionType, IBoundingBox,
 } from "../models/applicationState";
 import { ExportAssetState } from "../providers/export/exportProvider";
@@ -163,17 +163,17 @@ export default class MockFactory {
 
     /**
      * Creates array of fake IAsset
-     * @param count Number of assets to create
-     * @param videoFirst true if the first asset should be video; false otherwise
+     * @param count Number of assets to create (default: 10)
+     * @param startIndex The index that the assets should start at (default: 1)
      */
-    public static createTestAssets(count: number = 10, videoFirst: boolean = true): IAsset[] {
+    public static createTestAssets(count: number = 10, startIndex: number = 1): IAsset[] {
         const assets: IAsset[] = [];
-        for (let i = 1; i <= count; i++) {
-            assets.push((i % 2 === 1) ?
-                videoFirst ? MockFactory.createVideoTestAsset(i.toString()) :
-                    MockFactory.createTestAsset(i.toString()) :
-                !videoFirst ? MockFactory.createVideoTestAsset(i.toString()) :
-                    MockFactory.createTestAsset(i.toString()));
+        for (let i = startIndex; i < (count + startIndex); i++) {
+            const newAsset = (i % 2 === 1)
+                ? MockFactory.createVideoTestAsset(i.toString())
+                : MockFactory.createTestAsset(i.toString());
+
+            assets.push(newAsset);
         }
 
         return assets;

src/react/components/pages/editorPage/editorPage.test.tsx

@@ -1,5 +1,6 @@
-import { mount, ReactWrapper } from "enzyme";
 import React from "react";
+import { mount, ReactWrapper } from "enzyme";
+import _ from "lodash";
 import { Provider } from "react-redux";
 import { BrowserRouter as Router } from "react-router-dom";
 import { AnyAction, Store } from "redux";
@@ -125,8 +126,11 @@ describe("Editor Page Component", () => {
         expect(editorPage.prop("project")).toEqual(testProject);
     });
 
-    it("Loads project assets when state changes", async () => {
+    it("Loads and merges project assets with asset provider assets when state changes", async () => {
+        const projectAssets = MockFactory.createTestAssets(10, 10);
         const testProject = MockFactory.createTestProject("TestProject");
+        testProject.assets = _.keyBy(projectAssets, (asset) => asset.id);
+
         const store = createStore(testProject, true);
         const props = MockFactory.editorPageProps(testProject.id);
 
@@ -138,12 +142,12 @@ describe("Editor Page Component", () => {
             name: testProject.name,
         };
 
-        const expectedAssetMetadtata: IAssetMetadata = getMockAssetMetadata(testAssets);
-
         await MockFactory.flushUi();
 
+        const expectedAssetMetadtata: IAssetMetadata = getMockAssetMetadata(projectAssets);
+
         expect(editorPage.props().project).toEqual(expect.objectContaining(partialProject));
-        expect(editorPage.state().assets.length).toEqual(testAssets.length);
+        expect(editorPage.state().assets.length).toEqual(projectAssets.length + testAssets.length);
         expect(editorPage.state().selectedAsset).toMatchObject(expectedAssetMetadtata);
     });
 
@@ -193,14 +197,7 @@ describe("Editor Page Component", () => {
             const props = MockFactory.editorPageProps(testProject.id);
 
             wrapper = createComponent(store, props);
-
-            await MockFactory.waitForCondition(() => {
-                const editorPage = wrapper
-                    .find(EditorPage)
-                    .childAt(0);
-
-                return !!editorPage.state().selectedAsset;
-            });
+            await waitForSelectedAsset(wrapper);
         });
 
         it("editor mode is changed correctly", async () => {
@@ -292,14 +289,7 @@ describe("Editor Page Component", () => {
             });
 
             const wrapper = createComponent(store, MockFactory.editorPageProps());
-
-            await MockFactory.waitForCondition(() => {
-                const editorPage = wrapper
-                    .find(EditorPage)
-                    .childAt(0);
-
-                return !!editorPage.state().selectedAsset;
-            });
+            await waitForSelectedAsset(wrapper);
 
             wrapper.update();
 
@@ -328,3 +318,13 @@ function createStore(project: IProject, setCurrentProject: boolean = false): Sto
 
     return createReduxStore(initialState);
 }
+
+async function waitForSelectedAsset(wrapper: ReactWrapper) {
+    await MockFactory.waitForCondition(() => {
+        const editorPage = wrapper
+            .find(EditorPage)
+            .childAt(0);
+
+        return !!editorPage.state().selectedAsset;
+    });
+}

src/react/components/pages/editorPage/editorPage.tsx

@@ -271,9 +271,6 @@ export default class EditorPage extends React.Component<IEditorPageProps, IEdito
                 editorMode = EditorMode.CopyRect;
                 break;
             case "selectCanvas":
-                selectionMode = SelectionMode.NONE;
-                editorMode = EditorMode.Select;
-                break;
             case "panCanvas":
                 selectionMode = SelectionMode.NONE;
                 editorMode = EditorMode.Select;
@@ -311,7 +308,6 @@ export default class EditorPage extends React.Component<IEditorPageProps, IEdito
 
         this.setState({
             selectedAsset: assetMetadata,
-            assets: _.values(this.props.project.assets),
         });
     }
 
@@ -322,14 +318,18 @@ export default class EditorPage extends React.Component<IEditorPageProps, IEdito
 
         this.loadingProjectAssets = true;
 
-        await this.props.actions.loadAssets(this.props.project);
-        const assets = _.values(this.props.project.assets);
+        const projectAssets = _.values(this.props.project.assets);
+        const sourceAssets = await this.props.actions.loadAssets(this.props.project);
+        const allAssets = _(projectAssets)
+            .concat(sourceAssets)
+            .uniqBy((asset) => asset.id)
+            .value();
 
         this.setState({
-            assets,
+            assets: allAssets,
         }, async () => {
-            if (assets.length > 0) {
-                await this.selectAsset(assets[0]);
+            if (allAssets.length > 0) {
+                await this.selectAsset(allAssets[0]);
             }
             this.loadingProjectAssets = false;
         });

src/redux/reducers/currentProjectReducer.test.ts

@@ -11,6 +11,7 @@ import {
     saveAssetMetadataAction,
 } from "../actions/projectActions";
 import { anyOtherAction } from "../actions/actionCreators";
+import { saveConnectionAction } from "../actions/connectionActions";
 
 describe("Current Project Reducer", () => {
     it("Load project sets current project state", () => {
@@ -39,14 +40,39 @@ describe("Current Project Reducer", () => {
         expect(result).toBeNull();
     });
 
-    it("Load Project Assets merges assets into current asset set", () => {
+    it("Updating connection not in use by current project performs noop", () => {
+        const currentProject = MockFactory.createTestProject("1");
+        const state: IProject = currentProject;
+        const unrelatedConnection = MockFactory.createTestConnection("Unrelated Connection");
+        const action = saveConnectionAction(unrelatedConnection);
+        const result = reducer(state, action);
+        expect(result).toEqual(currentProject);
+    });
+
+    it("Updating connection used by current project is updated in curren project", () => {
+        const currentProject = MockFactory.createTestProject("1");
+        const state: IProject = currentProject;
+
+        const updatedConnection = { ...currentProject.sourceConnection };
+        updatedConnection.description += "updated";
+
+        const action = saveConnectionAction(updatedConnection);
+        const result = reducer(state, action);
+
+        expect(result).toEqual({
+            ...currentProject,
+            sourceConnection: updatedConnection,
+            targetConnection: updatedConnection,
+        });
+    });
+
+    it("Load Project Assets does not merges assets into current asset set", () => {
         const state: IProject = MockFactory.createTestProject("TestProject");
         const testAssets = MockFactory.createTestAssets();
 
         const action = loadProjectAssetsAction(testAssets);
         const result = reducer(state, action);
-        expect(result).not.toBe(state);
-        expect(Object.keys(result.assets).length).toEqual(testAssets.length);
+        expect(result).toBe(state);
     });
 
     it("Save Asset Metadata updates project asset state", () => {

src/redux/reducers/currentProjectReducer.ts

@@ -21,34 +21,32 @@ export const reducer = (state: IProject = null, action: AnyAction): IProject =>
             return null;
         case ActionTypes.LOAD_PROJECT_SUCCESS:
             return { ...action.payload };
-        case ActionTypes.LOAD_PROJECT_ASSETS_SUCCESS:
-            if (state) {
-                const currentAssets = { ...state.assets } || {};
-                action.payload.forEach((asset) => {
-                    if (!currentAssets[asset.id]) {
-                        currentAssets[asset.id] = asset;
-                    }
-                });
-
-                return {
-                    ...state,
-                    assets: currentAssets,
-                };
-            } else {
-                return state;
-            }
         case ActionTypes.SAVE_ASSET_METADATA_SUCCESS:
-            if (state) {
-                const updatedAssets = { ...state.assets } || {};
-                updatedAssets[action.payload.asset.id] = { ...action.payload.asset };
-
-                return {
-                    ...state,
-                    assets: updatedAssets,
-                };
-            } else {
+            if (!state) {
                 return state;
             }
+
+            const updatedAssets = { ...state.assets } || {};
+            updatedAssets[action.payload.asset.id] = { ...action.payload.asset };
+
+            return {
+                ...state,
+                assets: updatedAssets,
+            };
+        case ActionTypes.SAVE_CONNECTION_SUCCESS:
+            if (!state) {
+                return state;
+            }
+
+            return {
+                ...state,
+                sourceConnection: state.sourceConnection.id === action.payload.id
+                    ? { ...action.payload }
+                    : state.sourceConnection,
+                targetConnection: state.targetConnection.id === action.payload.id
+                    ? { ...action.payload }
+                    : state.targetConnection,
+            };
         default:
             return state;
     }

src/services/assetService.test.ts

@@ -1,5 +1,5 @@
 import { AssetService } from "./assetService";
-import { AssetType, IAssetMetadata } from "../models/applicationState";
+import { AssetType, IAssetMetadata, AssetState } from "../models/applicationState";
 import MockFactory from "../common/mockFactory";
 import { AssetProviderFactory, IAssetProvider } from "../providers/storage/assetProviderFactory";
 import { StorageProviderFactory, IStorageProvider } from "../providers/storage/storageProviderFactory";
@@ -118,7 +118,10 @@ describe("Asset Service", () => {
 
         it("Saves asset JSON to underlying storage provider", async () => {
             const assetMetadata: IAssetMetadata = {
-                asset: testAssets[0],
+                asset: {
+                    ...testAssets[0],
+                    state: AssetState.Tagged,
+                },
                 regions: [],
             };
 
@@ -131,6 +134,21 @@ describe("Asset Service", () => {
             expect(result).toBe(assetMetadata);
         });
 
+        it("Does not save asset JSON to the storage provider if asset has not been tagged", async () => {
+            const assetMetadata: IAssetMetadata = {
+                asset: {
+                    ...testAssets[0],
+                    state: AssetState.Visited,
+                },
+                regions: [],
+            };
+
+            const result = await assetService.save(assetMetadata);
+
+            expect(storageProviderMock.writeText).not.toBeCalled();
+            expect(result).toBe(assetMetadata);
+        });
+
         it("getAssets encodes local file path", async () => {
             const testAsset = MockFactory.createTestAsset(" 11");
             testAssets.push(testAsset);

src/services/assetService.ts

@@ -149,8 +149,12 @@ export class AssetService {
     public async save(metadata: IAssetMetadata): Promise<IAssetMetadata> {
         Guard.null(metadata);
 
-        const fileName = `${metadata.asset.id}${constants.assetMetadataFileExtension}`;
-        await this.storageProvider.writeText(fileName, JSON.stringify(metadata, null, 4));
+        // Only save asset metadata if asset is in a tagged state
+        // Otherwise primary asset information is already persisted in the project file.
+        if (metadata.asset.state === AssetState.Tagged) {
+            const fileName = `${metadata.asset.id}${constants.assetMetadataFileExtension}`;
+            await this.storageProvider.writeText(fileName, JSON.stringify(metadata, null, 4));
+        }
 
         return metadata;
     }