Mobile Stories block - unit tests for BlockMediaUpdateProgress #26423
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mzorz
added
[Type] Enhancement
|
Size Change: 0 B Total Size: 1.19 MB ℹ️ View Unchanged
|
| expect( onUpdateMediaUploadProgress ).toHaveBeenCalledTimes( 0 ); | ||
| } ); | ||
|
|
||
| it( 'upload: listens media upload success', () => { |
There was a problem hiding this comment.
@mzorz can we make these test descriptions a sentence/phrase without the upload: ? Let me know if it's a paradigm I am missing 🙏
There was a problem hiding this comment.
Could we also make the purpose of this test a bit more descriptive in what it does? I am seeing the description and logic at the expect calls and they don't correlate easily for me. Let me know 🙏
There was a problem hiding this comment.
can we make these test descriptions a sentence/phrase without the upload: ? Let me know if it's a paradigm I am missing 🙏
No paradigm missed here 👍 , it made sense since there's a very similar set of tests testing for the same conditions on two different operations: upload and save, that I prefixed each test with that. So I think it's clearer given the nature of the component which can handle progress updates on these two operations. I think it's fine to keep it, unless you feel strongly against that.
Could we also make the purpose of this test a bit more descriptive in what it does? I am seeing the description and logic at the expect calls and they don't correlate easily for me. Let me know
the name of the test is listens to media upload sucess - the assertions are about making sure the onFinishMediaUploadWithSuccess method gets called once and with the success test payload. Do you have anything in mind that better describes the purpose of the test? Let me know 🙏
There was a problem hiding this comment.
No paradigm missed here 👍 , it made sense since there's a very similar set of tests testing for the same conditions on two different operations: upload and save, that I prefixed each test with that. So I think it's clearer given the nature of the component which can handle progress updates on these two operations. I think it's fine to keep it, unless you feel strongly against that.
Understood! Yes, it's fine to keep it. 🤝
the name of the test is listens to media upload sucess - the assertions are about making sure the onFinishMediaUploadWithSuccess method gets called once and with the success test payload. Do you have anything in mind that better describes the purpose of the test? Let me know 🙏
Thanks for the explanation. I sometimes put the name of the method/target in the name of the method. I came up with this below
onFinishMediaUploadWithSuccess is called when a success payload is received
That's what I came up with :) If it's okay with you we could use it or keep the one that's there 🤝
There was a problem hiding this comment.
Nice! I changed that one and the rest to comply with the same naming pattern in 1f57e1d
packages/block-editor/src/components/block-media-update-progress/test/index.native.js
Show resolved
Hide resolved
packages/block-editor/src/components/block-media-update-progress/test/index.native.js
Outdated
Show resolved
Hide resolved
|
@guarani when you get a chance I would like your opinion on these tests as I am still getting up to speed with the paradigms and test syntax used in the project 😄 |
guarani
reviewed
Oct 28, 2020
packages/block-editor/src/components/block-media-update-progress/test/index.native.js
Outdated
Show resolved
Hide resolved
guarani
reviewed
Oct 28, 2020
packages/block-editor/src/components/block-media-update-progress/test/index.native.js
Outdated
Show resolved
Hide resolved
guarani
approved these changes
Oct 28, 2020
|
Thank you for your review @guarani @jd-alexander 🙇 |
mzorz
added a commit
that referenced
this pull request
Nov 10, 2020
* updated initial example html content file with story block * adding method requestStoryCreatorLoad() to the bridge * added new interface OnStoryCreatorRequestListener * passing mediaFiles and blockId over the bridge in requestStoryCreatorLoad() * prettier fix * added gray-700 color, missing if not after retiring and changes made in (#25213) * removed jetpack stories example block from initial demo load * Mobile Stories block (part2: on done) (#25771) * added mobile StoryUpdateProgress component and bridge code to send/receive save progress * updated WPAndroid bridge DeferredEventEmitter to handle Story save events separately * changed all Save event interface methods to use String ids instead of int, and removed serverMediaId params as these don't apply while saving locally * redefined upload/save state constants * added onStorySaveResult handling to bridge, and renamed STORY_SAVE_STATE_* events to MEDIA_SAVE_STATE_* where appropriate * checking for matches of mediaId in mediaFiles while saving to send save progress updates * added mediaModelCreated() method to the bridge, so a new ID can be assigned to a mediaFile in a Story block * mediaId should always be a string in mediaFiles so, converting to avoid strict comparison to fail * removed commented code * updated documentation * added missing implementation of method storySaveSync() in demo app * fixed prettier warning * Update packages/block-editor/src/components/story-update-progress/README.md Co-authored-by: Joel Dean <[email protected]> * Update packages/block-editor/src/components/story-update-progress/README.md Co-authored-by: Joel Dean <[email protected]> * Mobile Stories block (part3: refactor / rename) (#26005) * renames for generic media files collection block and BlockMediaUpdateProgres * referencing the right props method in finishMediaSaveWithFailure * mistaken renames of parameters in bridge methods * renamed more abstract/generic method names requestMediaFilesEditorLoad * removed extra whtie space * renamed argument type Co-authored-by: Joel Dean <[email protected]> * Update packages/block-editor/src/components/block-media-update-progress/README.md Co-authored-by: Paul Von Schrottky <[email protected]> * using array.some() to find element in array * prettified function call * removed commented code * Media Files Collection (part4: error handling) (#26008) * added mobile StoryUpdateProgress component and bridge code to send/receive save progress * updated WPAndroid bridge DeferredEventEmitter to handle Story save events separately * changed all Save event interface methods to use String ids instead of int, and removed serverMediaId params as these don't apply while saving locally * redefined upload/save state constants * added onStorySaveResult handling to bridge, and renamed STORY_SAVE_STATE_* events to MEDIA_SAVE_STATE_* where appropriate * checking for matches of mediaId in mediaFiles while saving to send save progress updates * added mediaModelCreated() method to the bridge, so a new ID can be assigned to a mediaFile in a Story block * mediaId should always be a string in mediaFiles so, converting to avoid strict comparison to fail * removed commented code * updated documentation * added missing implementation of method storySaveSync() in demo app * fixed prettier warning * renames for generic media files collection block and BlockMediaUpdateProgres * referencing the right props method in finishMediaSaveWithFailure * mistaken renames of parameters in bridge methods * renamed more abstract/generic method names requestMediaFilesEditorLoad * removed extra whtie space * renamed argument type * renamed event paramter name to easier to understand 'success' boolean, matching the native counterpart * added cancel and retry bridge methods specific for mediaFiles collection based blocks * added requestMediaFilesSaveCancelDialog bridge method * renamed bridge method mediaModelCreatedForFile for more general purpose mediaIdChanged * Add missing iOS bridge declarations * added jsdoc like description for methods * removed unneeded return statement in some bridge methods * Update packages/react-native-bridge/index.js Co-authored-by: Joel Dean <[email protected]> * Update packages/react-native-bridge/index.js Co-authored-by: Joel Dean <[email protected]> * Update packages/react-native-bridge/index.js Co-authored-by: Joel Dean <[email protected]> * fixed typo, added punctuation Co-authored-by: eToledo <[email protected]> Co-authored-by: Joel Dean <[email protected]> * [RNMobile] added jetpack node_modules folder to cleanup step in JSbundle building (#26247) * added mobile StoryUpdateProgress component and bridge code to send/receive save progress * updated WPAndroid bridge DeferredEventEmitter to handle Story save events separately * changed all Save event interface methods to use String ids instead of int, and removed serverMediaId params as these don't apply while saving locally * redefined upload/save state constants * added onStorySaveResult handling to bridge, and renamed STORY_SAVE_STATE_* events to MEDIA_SAVE_STATE_* where appropriate * checking for matches of mediaId in mediaFiles while saving to send save progress updates * added mediaModelCreated() method to the bridge, so a new ID can be assigned to a mediaFile in a Story block * mediaId should always be a string in mediaFiles so, converting to avoid strict comparison to fail * removed commented code * updated documentation * added missing implementation of method storySaveSync() in demo app * fixed prettier warning * renames for generic media files collection block and BlockMediaUpdateProgres * referencing the right props method in finishMediaSaveWithFailure * mistaken renames of parameters in bridge methods * renamed more abstract/generic method names requestMediaFilesEditorLoad * removed extra whtie space * renamed argument type * renamed event paramter name to easier to understand 'success' boolean, matching the native counterpart * added cancel and retry bridge methods specific for mediaFiles collection based blocks * added requestMediaFilesSaveCancelDialog bridge method * renamed bridge method mediaModelCreatedForFile for more general purpose mediaIdChanged * added jetpack node_modules folder to cleanup step in JSbundle building tasks * Mobile Stories block - unit tests for BlockMediaUpdateProgress (#26423) * adding upload progress tests on BLockMediaUpdateProgress component (mediaFiles collection) * added save event tests for BlockMediaUpdateProgress component * ids in mediaFiles array are always strings, fixed that * updated function name passed by props * fixed mediaFiles const passing for saving, using tempMediaFiles * fixed state change for saveReset signal, was changing upload state instead * fixed typo * fixed using save state var * changed to more descriptive test names * removed commented line * renamed vars to match bridge signal name * Mobile stories block: hide behind feature flag (#26522) * hide Story block in non-DEV builds * prettified * passing enableStories feature flag in GutenbergProps and hiding block from picker if false * added space * renamed bridge GutenbergProps property to agnostic abstraction mediaFilesCollectionBlock * renamed var on Capabilities enum * removed stories specific block behavior * added Jdoc descriptions * [RNMobile] iOS side of bridge for media collection blocks (#26704) * Activate media files collection capabilitie on iOS example app * Implement iOS side of Bridge for Media Collection blocks * Update packages/react-native-bridge/ios/RNReactNativeGutenbergBridge.swift Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: Joel Dean <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: eToledo <[email protected]>
cameronvoell
pushed a commit
that referenced
this pull request
Nov 14, 2020
* updated initial example html content file with story block * adding method requestStoryCreatorLoad() to the bridge * added new interface OnStoryCreatorRequestListener * passing mediaFiles and blockId over the bridge in requestStoryCreatorLoad() * prettier fix * added gray-700 color, missing if not after retiring and changes made in (#25213) * removed jetpack stories example block from initial demo load * Mobile Stories block (part2: on done) (#25771) * added mobile StoryUpdateProgress component and bridge code to send/receive save progress * updated WPAndroid bridge DeferredEventEmitter to handle Story save events separately * changed all Save event interface methods to use String ids instead of int, and removed serverMediaId params as these don't apply while saving locally * redefined upload/save state constants * added onStorySaveResult handling to bridge, and renamed STORY_SAVE_STATE_* events to MEDIA_SAVE_STATE_* where appropriate * checking for matches of mediaId in mediaFiles while saving to send save progress updates * added mediaModelCreated() method to the bridge, so a new ID can be assigned to a mediaFile in a Story block * mediaId should always be a string in mediaFiles so, converting to avoid strict comparison to fail * removed commented code * updated documentation * added missing implementation of method storySaveSync() in demo app * fixed prettier warning * Update packages/block-editor/src/components/story-update-progress/README.md Co-authored-by: Joel Dean <[email protected]> * Update packages/block-editor/src/components/story-update-progress/README.md Co-authored-by: Joel Dean <[email protected]> * Mobile Stories block (part3: refactor / rename) (#26005) * renames for generic media files collection block and BlockMediaUpdateProgres * referencing the right props method in finishMediaSaveWithFailure * mistaken renames of parameters in bridge methods * renamed more abstract/generic method names requestMediaFilesEditorLoad * removed extra whtie space * renamed argument type Co-authored-by: Joel Dean <[email protected]> * Update packages/block-editor/src/components/block-media-update-progress/README.md Co-authored-by: Paul Von Schrottky <[email protected]> * using array.some() to find element in array * prettified function call * removed commented code * Media Files Collection (part4: error handling) (#26008) * added mobile StoryUpdateProgress component and bridge code to send/receive save progress * updated WPAndroid bridge DeferredEventEmitter to handle Story save events separately * changed all Save event interface methods to use String ids instead of int, and removed serverMediaId params as these don't apply while saving locally * redefined upload/save state constants * added onStorySaveResult handling to bridge, and renamed STORY_SAVE_STATE_* events to MEDIA_SAVE_STATE_* where appropriate * checking for matches of mediaId in mediaFiles while saving to send save progress updates * added mediaModelCreated() method to the bridge, so a new ID can be assigned to a mediaFile in a Story block * mediaId should always be a string in mediaFiles so, converting to avoid strict comparison to fail * removed commented code * updated documentation * added missing implementation of method storySaveSync() in demo app * fixed prettier warning * renames for generic media files collection block and BlockMediaUpdateProgres * referencing the right props method in finishMediaSaveWithFailure * mistaken renames of parameters in bridge methods * renamed more abstract/generic method names requestMediaFilesEditorLoad * removed extra whtie space * renamed argument type * renamed event paramter name to easier to understand 'success' boolean, matching the native counterpart * added cancel and retry bridge methods specific for mediaFiles collection based blocks * added requestMediaFilesSaveCancelDialog bridge method * renamed bridge method mediaModelCreatedForFile for more general purpose mediaIdChanged * Add missing iOS bridge declarations * added jsdoc like description for methods * removed unneeded return statement in some bridge methods * Update packages/react-native-bridge/index.js Co-authored-by: Joel Dean <[email protected]> * Update packages/react-native-bridge/index.js Co-authored-by: Joel Dean <[email protected]> * Update packages/react-native-bridge/index.js Co-authored-by: Joel Dean <[email protected]> * fixed typo, added punctuation Co-authored-by: eToledo <[email protected]> Co-authored-by: Joel Dean <[email protected]> * [RNMobile] added jetpack node_modules folder to cleanup step in JSbundle building (#26247) * added mobile StoryUpdateProgress component and bridge code to send/receive save progress * updated WPAndroid bridge DeferredEventEmitter to handle Story save events separately * changed all Save event interface methods to use String ids instead of int, and removed serverMediaId params as these don't apply while saving locally * redefined upload/save state constants * added onStorySaveResult handling to bridge, and renamed STORY_SAVE_STATE_* events to MEDIA_SAVE_STATE_* where appropriate * checking for matches of mediaId in mediaFiles while saving to send save progress updates * added mediaModelCreated() method to the bridge, so a new ID can be assigned to a mediaFile in a Story block * mediaId should always be a string in mediaFiles so, converting to avoid strict comparison to fail * removed commented code * updated documentation * added missing implementation of method storySaveSync() in demo app * fixed prettier warning * renames for generic media files collection block and BlockMediaUpdateProgres * referencing the right props method in finishMediaSaveWithFailure * mistaken renames of parameters in bridge methods * renamed more abstract/generic method names requestMediaFilesEditorLoad * removed extra whtie space * renamed argument type * renamed event paramter name to easier to understand 'success' boolean, matching the native counterpart * added cancel and retry bridge methods specific for mediaFiles collection based blocks * added requestMediaFilesSaveCancelDialog bridge method * renamed bridge method mediaModelCreatedForFile for more general purpose mediaIdChanged * added jetpack node_modules folder to cleanup step in JSbundle building tasks * Mobile Stories block - unit tests for BlockMediaUpdateProgress (#26423) * adding upload progress tests on BLockMediaUpdateProgress component (mediaFiles collection) * added save event tests for BlockMediaUpdateProgress component * ids in mediaFiles array are always strings, fixed that * updated function name passed by props * fixed mediaFiles const passing for saving, using tempMediaFiles * fixed state change for saveReset signal, was changing upload state instead * fixed typo * fixed using save state var * changed to more descriptive test names * removed commented line * renamed vars to match bridge signal name * Mobile stories block: hide behind feature flag (#26522) * hide Story block in non-DEV builds * prettified * passing enableStories feature flag in GutenbergProps and hiding block from picker if false * added space * renamed bridge GutenbergProps property to agnostic abstraction mediaFilesCollectionBlock * renamed var on Capabilities enum * removed stories specific block behavior * added Jdoc descriptions * [RNMobile] iOS side of bridge for media collection blocks (#26704) * Activate media files collection capabilitie on iOS example app * Implement iOS side of Bridge for Media Collection blocks * Update packages/react-native-bridge/ios/RNReactNativeGutenbergBridge.swift Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: Joel Dean <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: eToledo <[email protected]>
mzorz
added a commit
that referenced
this pull request
Nov 14, 2020
* updated initial example html content file with story block * adding method requestStoryCreatorLoad() to the bridge * added new interface OnStoryCreatorRequestListener * passing mediaFiles and blockId over the bridge in requestStoryCreatorLoad() * prettier fix * added gray-700 color, missing if not after retiring and changes made in (#25213) * removed jetpack stories example block from initial demo load * Mobile Stories block (part2: on done) (#25771) * added mobile StoryUpdateProgress component and bridge code to send/receive save progress * updated WPAndroid bridge DeferredEventEmitter to handle Story save events separately * changed all Save event interface methods to use String ids instead of int, and removed serverMediaId params as these don't apply while saving locally * redefined upload/save state constants * added onStorySaveResult handling to bridge, and renamed STORY_SAVE_STATE_* events to MEDIA_SAVE_STATE_* where appropriate * checking for matches of mediaId in mediaFiles while saving to send save progress updates * added mediaModelCreated() method to the bridge, so a new ID can be assigned to a mediaFile in a Story block * mediaId should always be a string in mediaFiles so, converting to avoid strict comparison to fail * removed commented code * updated documentation * added missing implementation of method storySaveSync() in demo app * fixed prettier warning * Update packages/block-editor/src/components/story-update-progress/README.md Co-authored-by: Joel Dean <[email protected]> * Update packages/block-editor/src/components/story-update-progress/README.md Co-authored-by: Joel Dean <[email protected]> * Mobile Stories block (part3: refactor / rename) (#26005) * renames for generic media files collection block and BlockMediaUpdateProgres * referencing the right props method in finishMediaSaveWithFailure * mistaken renames of parameters in bridge methods * renamed more abstract/generic method names requestMediaFilesEditorLoad * removed extra whtie space * renamed argument type Co-authored-by: Joel Dean <[email protected]> * Update packages/block-editor/src/components/block-media-update-progress/README.md Co-authored-by: Paul Von Schrottky <[email protected]> * using array.some() to find element in array * prettified function call * removed commented code * Media Files Collection (part4: error handling) (#26008) * added mobile StoryUpdateProgress component and bridge code to send/receive save progress * updated WPAndroid bridge DeferredEventEmitter to handle Story save events separately * changed all Save event interface methods to use String ids instead of int, and removed serverMediaId params as these don't apply while saving locally * redefined upload/save state constants * added onStorySaveResult handling to bridge, and renamed STORY_SAVE_STATE_* events to MEDIA_SAVE_STATE_* where appropriate * checking for matches of mediaId in mediaFiles while saving to send save progress updates * added mediaModelCreated() method to the bridge, so a new ID can be assigned to a mediaFile in a Story block * mediaId should always be a string in mediaFiles so, converting to avoid strict comparison to fail * removed commented code * updated documentation * added missing implementation of method storySaveSync() in demo app * fixed prettier warning * renames for generic media files collection block and BlockMediaUpdateProgres * referencing the right props method in finishMediaSaveWithFailure * mistaken renames of parameters in bridge methods * renamed more abstract/generic method names requestMediaFilesEditorLoad * removed extra whtie space * renamed argument type * renamed event paramter name to easier to understand 'success' boolean, matching the native counterpart * added cancel and retry bridge methods specific for mediaFiles collection based blocks * added requestMediaFilesSaveCancelDialog bridge method * renamed bridge method mediaModelCreatedForFile for more general purpose mediaIdChanged * Add missing iOS bridge declarations * added jsdoc like description for methods * removed unneeded return statement in some bridge methods * Update packages/react-native-bridge/index.js Co-authored-by: Joel Dean <[email protected]> * Update packages/react-native-bridge/index.js Co-authored-by: Joel Dean <[email protected]> * Update packages/react-native-bridge/index.js Co-authored-by: Joel Dean <[email protected]> * fixed typo, added punctuation Co-authored-by: eToledo <[email protected]> Co-authored-by: Joel Dean <[email protected]> * [RNMobile] added jetpack node_modules folder to cleanup step in JSbundle building (#26247) * added mobile StoryUpdateProgress component and bridge code to send/receive save progress * updated WPAndroid bridge DeferredEventEmitter to handle Story save events separately * changed all Save event interface methods to use String ids instead of int, and removed serverMediaId params as these don't apply while saving locally * redefined upload/save state constants * added onStorySaveResult handling to bridge, and renamed STORY_SAVE_STATE_* events to MEDIA_SAVE_STATE_* where appropriate * checking for matches of mediaId in mediaFiles while saving to send save progress updates * added mediaModelCreated() method to the bridge, so a new ID can be assigned to a mediaFile in a Story block * mediaId should always be a string in mediaFiles so, converting to avoid strict comparison to fail * removed commented code * updated documentation * added missing implementation of method storySaveSync() in demo app * fixed prettier warning * renames for generic media files collection block and BlockMediaUpdateProgres * referencing the right props method in finishMediaSaveWithFailure * mistaken renames of parameters in bridge methods * renamed more abstract/generic method names requestMediaFilesEditorLoad * removed extra whtie space * renamed argument type * renamed event paramter name to easier to understand 'success' boolean, matching the native counterpart * added cancel and retry bridge methods specific for mediaFiles collection based blocks * added requestMediaFilesSaveCancelDialog bridge method * renamed bridge method mediaModelCreatedForFile for more general purpose mediaIdChanged * added jetpack node_modules folder to cleanup step in JSbundle building tasks * Mobile Stories block - unit tests for BlockMediaUpdateProgress (#26423) * adding upload progress tests on BLockMediaUpdateProgress component (mediaFiles collection) * added save event tests for BlockMediaUpdateProgress component * ids in mediaFiles array are always strings, fixed that * updated function name passed by props * fixed mediaFiles const passing for saving, using tempMediaFiles * fixed state change for saveReset signal, was changing upload state instead * fixed typo * fixed using save state var * changed to more descriptive test names * removed commented line * renamed vars to match bridge signal name * Mobile stories block: hide behind feature flag (#26522) * hide Story block in non-DEV builds * prettified * passing enableStories feature flag in GutenbergProps and hiding block from picker if false * added space * renamed bridge GutenbergProps property to agnostic abstraction mediaFilesCollectionBlock * renamed var on Capabilities enum * removed stories specific block behavior * added Jdoc descriptions * [RNMobile] iOS side of bridge for media collection blocks (#26704) * Activate media files collection capabilitie on iOS example app * Implement iOS side of Bridge for Media Collection blocks * Update packages/react-native-bridge/ios/RNReactNativeGutenbergBridge.swift Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: Joel Dean <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: eToledo <[email protected]> Co-authored-by: mzorz <[email protected]> Co-authored-by: Joel Dean <[email protected]> Co-authored-by: Paul Von Schrottky <[email protected]> Co-authored-by: eToledo <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds unit tests for the BlockMediaUpdateProgress component introduced in #25242
Based on pre-existing tests for MediaUploadProgress component, and adapted to our own
mediaFilescollection use cases.