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.

Sign up for GitHub

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

Jump to bottom

Fixed crash when trying to reupload nonexistent media. #10791

Merged
merged 2 commits into from
Nov 20, 2019
Merged

Fixed crash when trying to reupload nonexistent media. #10791

merged 2 commits into from
Nov 20, 2019

Conversation

khaykov
Copy link
Member

@khaykov khaykov commented Nov 13, 2019 •
edited

Fixes #10609

When removing an image that failed to upload, we were not removing it from the failed media list, which we use to reupload all media.

To test:

  • Turn on airplane mode.
  • Start a new post with Block editor.
  • Add an image block and add an image from the device.
  • Add another image block and add another image from the device.
  • Tap on the last image and remove it.
  • Tap on the first image and select "RETRY ALL" from the dialog.
  • Notice that the app is not crashing.

PR submission checklist:

  • I have considered adding unit tests where possible.

  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@khaykov khaykov added [Type] Bug [Type] Crash Gutenberg Editing and display of Gutenberg blocks. labels Nov 13, 2019
@khaykov khaykov added this to the 13.7 milestone Nov 13, 2019
@khaykov khaykov requested review from maxme and mzorz November 13, 2019 22:44
@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Nov 13, 2019 •
edited

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

@jkmassel
Copy link
Contributor

Howdy folks! I'm freezing 13.7 today, so this is being bumped to 13.8. If you'd like it to go out as part of 13.7, please target release/13.7 for this PR, and DM me once it's merged – I'll be happy to cut a new beta release!

@jkmassel jkmassel modified the milestones: 13.7, 13.8 Nov 18, 2019
@mzorz mzorz self-assigned this Nov 19, 2019
mzorz
mzorz reviewed Nov 19, 2019
View reviewed changes
...itor/WordPressEditor/src/main/java/org/wordpress/android/editor/GutenbergEditorFragment.java
@@ -509,6 +509,7 @@ public void onClick(DialogInterface dialog, int id) {
public void onClick(DialogInterface dialog, int id) {
dialog.dismiss();
mEditorFragmentListener.onMediaDeleted(String.valueOf(mediaId));
mFailedMediaIds.remove(String.valueOf(mediaId));
Copy link
Contributor

Choose a reason for hiding this comment

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

Note (unrelated) for clarity: while testing this PR step by step, I noticed we should also be doing this when the user taps on the trash bin icon to delete the block (as opposed to "removing" the image from the block, without removing the actual empty image block).
Given that action isn't related to what this PR specifically fixes, going to check and open a new issue separately if merits.

Copy link
Contributor

Choose a reason for hiding this comment

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

Tracked this in wordpress-mobile/gutenberg-mobile#1595

mzorz
mzorz approved these changes Nov 19, 2019
View reviewed changes
Copy link
Contributor

@mzorz mzorz left a comment

Choose a reason for hiding this comment

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

I confirm the described issue is gone after applying this PR 👍 - should be good to go after resolving the conflicts with the target branch @khaykov, approving

@mzorz mzorz mentioned this pull request Nov 19, 2019
Closed
@mzorz mzorz merged commit 7b9a170 into develop Nov 20, 2019
@mzorz mzorz deleted the issue/10609-fixing-media-retry-crash branch November 20, 2019 13:06
@sentry-io
Copy link

sentry-io bot commented Dec 3, 2019

Sentry issue: WORDPRESS-ANDROID-79M

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mzorz mzorz mzorz approved these changes

@maxme maxme Awaiting requested review from maxme

Assignees

@mzorz mzorz

Labels
Gutenberg Editing and display of Gutenberg blocks. [Type] Bug [Type] Crash
Projects
None yet
Milestone
13.8
3 participants
@khaykov @jkmassel @mzorz