From 4c4a4d2c467cdc21df5a37726432556e7117e922 Mon Sep 17 00:00:00 2001 From: Erin Date: Tue, 20 Feb 2018 10:43:13 -0800 Subject: [PATCH 1/4] Undo version routing change that caused errors --- apps/src/gamelab/animationListModule.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/apps/src/gamelab/animationListModule.js b/apps/src/gamelab/animationListModule.js index 31dd5fb205ee1..53d7368d2cbca 100644 --- a/apps/src/gamelab/animationListModule.js +++ b/apps/src/gamelab/animationListModule.js @@ -709,18 +709,16 @@ export function animationSourceUrl(key, props, withVersion = false) { // 1. If the animation has a sourceUrl it's external (from the library // or some other outside source, not the animation API) - and we may need - // to run it through the media proxy. (Note - Before 02/2018 - - // uploaded images may/may not have non-null sourceUrls. After 02/2018 - - // uploaded images will have null sourceUrls) + // to run it through the media proxy. + if (props.sourceUrl) { + return assetPrefix.fixPath(props.sourceUrl); + } + // 2. Otherwise it's local to this project, and we should use the animation // key to look it up in the animations API. - let url = (props.sourceUrl) ? - assetPrefix.fixPath(props.sourceUrl) : (animationsApi.basePath(key) + '.png'); - - // Appending version here to support projects with uploaded images - // with sourceUrls. - return url + ((props.version) ? '?version=' + props.version : ''); -} + return animationsApi.basePath(key) + '.png' + + ((withVersion && props.version) ? '?version=' + props.version : ''); + } /** * Static helper for converting a serialized animation list to an exportable one From e2e2c4c1e7eef32221006be4d23cc26cadc2a762 Mon Sep 17 00:00:00 2001 From: Erin Date: Wed, 21 Feb 2018 11:15:44 -0800 Subject: [PATCH 2/4] Test that sourceUrl is null in uploaded images --- .../AnimationPicker/animationPickerModule.js | 34 +++++++++++-------- .../animationPickerModuleTest.js | 23 +++++++++++++ 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/apps/src/gamelab/AnimationPicker/animationPickerModule.js b/apps/src/gamelab/AnimationPicker/animationPickerModule.js index b1874cf1d09d9..5629b3a5d091d 100644 --- a/apps/src/gamelab/AnimationPicker/animationPickerModule.js +++ b/apps/src/gamelab/AnimationPicker/animationPickerModule.js @@ -111,30 +111,34 @@ export function handleUploadComplete(result) { const key = result.filename.replace(/\.png$/i, ''); const sourceUrl = animationsApi.basePath(key + '.png'); + const onImageMetadataLoaded = buildOnImageMetadataLoaded(uploadFilename, goal, key, result, dispatch); // TODO (bbuchanan): This sequencing feels backwards. Eventually, we // ought to preview and get dimensions from the local filesystem, async // with the upload itself, but that will mean refactoring away from the // jQuery uploader. - loadImageMetadata(sourceUrl, metadata => { - const animation = _.assign({}, metadata, { - name: uploadFilename, - sourceUrl: null, - size: result.size, - version: result.versionId - }); - - if (goal === Goal.NEW_ANIMATION) { - dispatch(addAnimation(key, animation)); - } else if (goal === Goal.NEW_FRAME) { - dispatch(appendCustomFrames(animation)); - } - dispatch(hide()); - }, () => { + loadImageMetadata(sourceUrl, onImageMetadataLoaded, () => { dispatch(handleUploadError(gamelabMsg.animationPicker_failedToParseImage())); }); }; } +export function buildOnImageMetadataLoaded(uploadFilename, goal, key, result, dispatch) { + return (metadata) => { + const animation = _.assign({}, metadata, { + name: uploadFilename, + sourceUrl: null, + size: result.size, + version: result.versionId + }); + if (goal === Goal.NEW_ANIMATION) { + dispatch(addAnimation(key, animation)); + } else if (goal === Goal.NEW_FRAME) { + dispatch(appendCustomFrames(animation)); + } + dispatch(hide()); + }; +} + /** * Asynchronously loads an image file as an Image, then derives appropriate * animation metadata from that Image and returns the metadata to a callback. diff --git a/apps/test/unit/gamelab/AnimationPicker/animationPickerModuleTest.js b/apps/test/unit/gamelab/AnimationPicker/animationPickerModuleTest.js index 8a91ac29ce98b..98b1a17512605 100644 --- a/apps/test/unit/gamelab/AnimationPicker/animationPickerModuleTest.js +++ b/apps/test/unit/gamelab/AnimationPicker/animationPickerModuleTest.js @@ -1,4 +1,7 @@ import reducer, * as animationPickerModule from '@cdo/apps/gamelab/AnimationPicker/animationPickerModule'; +import listReducer from '@cdo/apps/gamelab/animationListModule'; +import {combineReducers} from 'redux'; +import {createStore} from '../../../util/redux'; import {expect} from '../../../util/configuredChai'; var Goal = animationPickerModule.Goal; @@ -102,5 +105,25 @@ describe('animationPickerModule', function () { expect(newState.uploadError).to.equal(status); }); }); + + describe('action: handleUploadComplete', function () { + + it('sets sourceUrl to null', function () { + const newState = {animationPicker: { uploadFilename: "filename.jpg", goal: Goal.NEW_ANIMATION }}; + var store = createStore(combineReducers({animationList: listReducer, animationPicker: reducer}), newState); + + var onMetadataLoaded = animationPickerModule.buildOnImageMetadataLoaded( + "filename.jpg", Goal.NEW_ANIMATION, "filename.jpg", + {filename: "filename.jpg", result: 0, versionId: "string"}, store.dispatch); + onMetadataLoaded({}); + + const newListState = store.getState().animationList; + const animationKey = newListState.orderedKeys[0]; + + expect(newListState.propsByKey[animationKey]).to.be.not.null; + expect(newListState.propsByKey[animationKey].name).to.equal("filename.jpg"); + expect(newListState.propsByKey[animationKey].sourceUrl).to.be.null; + }); + }); }); }); From 416bb35b16c5c40d80589ddbfcfe6898dbadd9d5 Mon Sep 17 00:00:00 2001 From: Erin Date: Wed, 21 Feb 2018 14:11:36 -0800 Subject: [PATCH 3/4] Styling changes and removed redundent parameters --- .../gamelab/AnimationPicker/animationPickerModule.js | 10 +++------- .../AnimationPicker/animationPickerModuleTest.js | 11 +++++++++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/apps/src/gamelab/AnimationPicker/animationPickerModule.js b/apps/src/gamelab/AnimationPicker/animationPickerModule.js index 1870a28d8b51f..ca46f38e53a41 100644 --- a/apps/src/gamelab/AnimationPicker/animationPickerModule.js +++ b/apps/src/gamelab/AnimationPicker/animationPickerModule.js @@ -107,22 +107,18 @@ export function beginUpload(filename) { */ export function handleUploadComplete(result) { return function (dispatch, getState) { - const { goal, uploadFilename } = getState().animationPicker; const key = result.filename.replace(/\.png$/i, ''); const sourceUrl = animationsApi.basePath(key + '.png'); - const onImageMetadataLoaded = buildOnImageMetadataLoaded(uploadFilename, goal, key, result, dispatch); - // TODO (bbuchanan): This sequencing feels backwards. Eventually, we - // ought to preview and get dimensions from the local filesystem, async - // with the upload itself, but that will mean refactoring away from the - // jQuery uploader. + const onImageMetadataLoaded = buildOnImageMetadataLoaded(key, result, dispatch, getState); loadImageMetadata(sourceUrl, onImageMetadataLoaded, () => { dispatch(handleUploadError(gamelabMsg.animationPicker_failedToParseImage())); }); }; } -export function buildOnImageMetadataLoaded(uploadFilename, goal, key, result, dispatch) { +export function buildOnImageMetadataLoaded(key, result, dispatch, getState) { return (metadata) => { + const { goal, uploadFilename } = getState().animationPicker; const animation = _.assign({}, metadata, { name: uploadFilename, sourceUrl: null, diff --git a/apps/test/unit/gamelab/AnimationPicker/animationPickerModuleTest.js b/apps/test/unit/gamelab/AnimationPicker/animationPickerModuleTest.js index ff08f80db4f0f..1693e1086833e 100644 --- a/apps/test/unit/gamelab/AnimationPicker/animationPickerModuleTest.js +++ b/apps/test/unit/gamelab/AnimationPicker/animationPickerModuleTest.js @@ -113,8 +113,15 @@ describe('animationPickerModule', function () { var store = createStore(combineReducers({animationList: listReducer, animationPicker: reducer}), newState); var onMetadataLoaded = animationPickerModule.buildOnImageMetadataLoaded( - "filename.jpg", Goal.NEW_ANIMATION, "filename.jpg", - {filename: "filename.jpg", result: 0, versionId: "string"}, store.dispatch); + "filename.jpg", + { + filename: "filename.jpg", + result: 0, + versionId: "string" + }, + store.dispatch, + store.getState + ); onMetadataLoaded({}); const newListState = store.getState().animationList; From 349d7c2d2bae7da91a96a8a610f75a73a36bddb0 Mon Sep 17 00:00:00 2001 From: Brad Buchanan Date: Mon, 26 Feb 2018 14:42:49 -0800 Subject: [PATCH 4/4] Test callback in upload image sets sourceUrl null --- .../animationPickerModuleTest.js | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/apps/test/unit/gamelab/AnimationPicker/animationPickerModuleTest.js b/apps/test/unit/gamelab/AnimationPicker/animationPickerModuleTest.js index 1693e1086833e..5896c0da61dc2 100644 --- a/apps/test/unit/gamelab/AnimationPicker/animationPickerModuleTest.js +++ b/apps/test/unit/gamelab/AnimationPicker/animationPickerModuleTest.js @@ -1,3 +1,4 @@ +import sinon from 'sinon'; import reducer, * as animationPickerModule from '@cdo/apps/gamelab/AnimationPicker/animationPickerModule'; import listReducer from '@cdo/apps/gamelab/animationListModule'; import {combineReducers} from 'redux'; @@ -109,6 +110,7 @@ describe('animationPickerModule', function () { describe('action: handleUploadComplete', function () { it('sets sourceUrl to null', function () { + const fakeDispatch = sinon.spy(); const newState = {animationPicker: { uploadFilename: "filename.jpg", goal: Goal.NEW_ANIMATION }}; var store = createStore(combineReducers({animationList: listReducer, animationPicker: reducer}), newState); @@ -119,17 +121,30 @@ describe('animationPickerModule', function () { result: 0, versionId: "string" }, - store.dispatch, + fakeDispatch, store.getState ); onMetadataLoaded({}); - - const newListState = store.getState().animationList; - const animationKey = newListState.orderedKeys[0]; - - expect(newListState.propsByKey[animationKey]).to.be.not.null; - expect(newListState.propsByKey[animationKey].name).to.equal("filename.jpg_1"); - expect(newListState.propsByKey[animationKey].sourceUrl).to.be.null; + expect(fakeDispatch).to.have.been.calledTwice; + + const addAnimation = fakeDispatch.firstCall.args[0]; + expect(addAnimation).to.be.a('function'); + expect(fakeDispatch.secondCall.args[0]).to.deep.equal(animationPickerModule.hide()); + fakeDispatch.reset(); + + addAnimation(fakeDispatch, store.getState); + expect(fakeDispatch.firstCall.args[0]).to.deep.equal({ + type: 'AnimationList/ADD_ANIMATION', + key: 'filename.jpg', + props: + { + name: 'filename.jpg', + sourceUrl: null, + size: undefined, + version: 'string', + looping: true + } + }); }); }); });