Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mobile stories block - error handling 4.1 - move code out of EditPostActivity #13198

Conversation

mzorz
Copy link
Contributor

@mzorz mzorz commented Oct 21, 2020

Comes from https://github.com/wordpress-mobile/WordPress-Android/pull/13103/files#r508824133

This PR moves logic for Stories-related load/retry/cancel handling from EditPostActivity to dedicated class StoriesEventListener

To test:
Follow up on #13103 test steps

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

…ostActivity to dedicated class StoriesEventListener
@mzorz mzorz added [Type] Enhancement Gutenberg Editing and display of Gutenberg blocks. gutenberg-mobile labels Oct 21, 2020
@mzorz mzorz added this to the 16.1 milestone Oct 21, 2020
@mzorz mzorz requested a review from jd-alexander October 21, 2020 14:44
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Oct 21, 2020

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Oct 21, 2020

You can test the changes on this Pull Request by downloading the APK here.

@jd-alexander
Copy link
Contributor

@mzorz this is probably not related to this PR but I noticed that the error notification that gets created when a story isn't saved remains even after the save was successful so we probably want to cancel that notification in the logic somewhere. Let me know if you are able to reproduce. I was using CASE A: error in saves

@mzorz
Copy link
Contributor Author

mzorz commented Oct 22, 2020

@mzorz this is probably not related to this PR but I noticed that the error notification that gets created when a story isn't saved remains even after the save was successful so we probably want to cancel that notification in the logic somewhere. Let me know if you are able to reproduce. I was using CASE A: error in saves

Thank you @jd-alexander - tracked here Automattic/stories-android#579

@jd-alexander
Copy link
Contributor

I was retesting Case B and got this error during the flow:

   kotlin.UninitializedPropertyAccessException: lateinit property editorMediaListener has not been initialized
        at org.wordpress.android.ui.posts.editor.media.EditorMedia.access$getEditorMediaListener$p(EditorMedia.kt:52)
        at org.wordpress.android.ui.posts.editor.media.EditorMedia$retryFailedMediaAsync$1.invokeSuspend(EditorMedia.kt:233)

This occurred after:

tap on the story block failed overlay and observe the dialog appears "Couldn't upload file"
tap RETRY

I relaunched the app and the upload continued flawlessly. This might have been due to some emulator issue. Not sure but sharing.

Copy link
Contributor

@jd-alexander jd-alexander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these changes @mzorz the error resolution and overall fallback mechanisms of your logic and the existing logic in the UploadService works really well. LGTM 🚢 once you are fine with this.

@mzorz
Copy link
Contributor Author

mzorz commented Oct 22, 2020

Thanks for your tests and the reported error! Will now merge this one and the base one 👍

@mzorz mzorz merged commit 47902d1 into try/jetpack-stories-block-error-handling Oct 22, 2020
@mzorz mzorz deleted the try/stories-listener-move-from-editpostactivity branch October 22, 2020 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gutenberg Editing and display of Gutenberg blocks. [Type] Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants