Gutenberg - async media upload Post background processing #9132

mzorz · 2019-01-28T00:45:31Z

Fixes part of wordpress-mobile/gutenberg-mobile#475

Follow up to #9129 (media upload progress reattach). That PR handles the visual updates, while this other PR handles the updates to a Gutenberg Post when an image upload finishes but the user is not currently using the Gutenberg editor.

This PR introduces a MediaUploadReadyProcessor implementation for Gutenberg Image blocks based on search & replace, and also makes Post<-> media associations aware of Gutenberg posts in purgeMediaToPostAssociationsIfNotInPostAnymore.

For the first (MediaUploadReadyProcessor), I've been considering some possible paths and have made up my mind for the easiest to implement for now, but may as well not be the most reliable as it makes several assumptions regarding the structure of an Image block as it currently is.

However, the PR works as per the current state of affairs, and I believe we shouldn't expect there to be breaking changes to the block structure in the future given it's one of the core blocks.

For further discussion

Other possibilities:

Use React Native's headless JS https://facebook.github.io/react-native/docs/headless-js-android and get an instance of Gutenberg parser there somehow (this is a path that needs more investigation), special attention to this:

By default, your app will crash if you try to run a task while the app is in the foreground. This is to prevent developers from shooting themselves in the foot by doing a lot of work in a task and slowing the UI. You can pass a fourth boolean argument to control this behaviour.

(this is indeed to be used only when the GB Editor is not on the foreground, but we should also consider situations in which we have already launched the background task and the user is also trying to bring GB editor up in the middle.)

use an HTML parser at least to make sense of the HTML tags to make url replacement a little bit more intelligent (or, HTML aware)
use some kind of regex or string replacement like this one here.

To test:

Pretty much we can follow the test cases described in #5780, writing here the most simple case (situation described in CASE B there):

start a new draft
insert an image block and choose an from device
tap the back arrow to exit the editor, observe the image continues to upload
also tap back again to exit the Posts list, back to the main app screen (as the posts list screen gets updated more frequently and can change the effect of this test). Even send the app to the background if you want.
once the image is done uploading a notification will appear.
go to the web and open the Post
confirm the image block has no validation errors, and the Post opens and displays the image correctly.

Update release notes:

If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

…onsIfNotInPostAnymore in GB

mzorz · 2019-01-28T00:46:38Z

WordPress/src/main/java/org/wordpress/android/ui/uploads/MediaUploadReadyProcessor.java


-            if (showAztecEditor) {
+            if (showGutenbergEditor) {
+                // TODO check if anything needs be done in Gutenberg


This // TODO mark left here on purpose

…_background_process

marecar3 · 2019-01-29T16:56:55Z

...itor/WordPressEditor/src/main/java/org/wordpress/android/editor/GutenbergEditorFragment.java

+                        Matcher m = p.matcher(postContent.substring(iStartOfImgTag));
+                        if (m.find()) {
+                            String src = m.group();
+                            int startIndex = src.indexOf("src=") + 5;


Can we put magic number 5 in some variable so that we can remember what's the idea behind it ?

👍 addressed in 6c5ec0a

er, re-wrote it in 924725e to a class constant so it's easier to read

…ground_process

marecar3 · 2019-01-29T18:37:55Z

...itor/WordPressEditor/src/main/java/org/wordpress/android/editor/GutenbergEditorFragment.java

+            // TODO: replace the URL
+            if (!mediaFile.isVideo()) {
+                // replace gutenberg block id holder with serverMediaId, and url_holder with remoteUrl
+                String oldImgBlockHeader = String.format("<!-- wp:image {\"id\":%s} -->", localMediaId);


Probably we can put this  in some constant ?

addressed in a321f5e

marecar3 · 2019-01-29T18:38:48Z

...itor/WordPressEditor/src/main/java/org/wordpress/android/editor/GutenbergEditorFragment.java

@@ -368,6 +371,55 @@ public static boolean contentContainsGutenbergBlocks(String postContent) {
        return (postContent != null && postContent.contains(GUTENBERG_BLOCK_START));
    }

+    public static String replaceMediaFileWithUrl(Context context, @NonNull String postContent,


As this is a static method, maybe it's a good candidate for some Util or Helper?

That's a good idea, I was following the same pattern as we had for Aztec: in Aztec the class that can handle / is responsible for editing a Post is AztecEditorFragment, and we needed a static method as this manipulation of the Post's content happens asynchronously (that is, when the AztecEditorFragment is not instanced).
For Gutenberg instead, this responsibility can be split into another class as you suggested.

addressed this change in 7472379

…gment as it was already moved to PostUtils before

mzorz · 2019-01-29T19:46:32Z

Thanks for your feedback, this is ready for another round @marecar3 🙇

marecar3

Nice Job @mzorz!

mzorz added 2 commits January 27, 2019 21:18

added background remoteurl processing for finished uploads on Gutenberg

abce82c

added Gutenberg find for image to implement purgeMediaToPostAssociati…

9ed2664

…onsIfNotInPostAnymore in GB

mzorz added Media gutenberg-mobile labels Jan 28, 2019

mzorz added this to the 11.8 milestone Jan 28, 2019

mzorz requested review from SergioEstevao, hypest and marecar3 January 28, 2019 00:45

mzorz commented Jan 28, 2019

View reviewed changes

mzorz added 2 commits January 27, 2019 21:48

fixed lint warning

7eb0760

removed old comment

75ddb4b

mzorz mentioned this pull request Jan 28, 2019

Attach PostUploader statuses to media upload in Gutenberg wordpress-mobile/gutenberg-mobile#475

Closed

2 tasks

mzorz added 3 commits January 28, 2019 08:29

updated gutenberg-mobile hash

144c93c

Merge branch 'gb/media_upload_progress_reattach' into gb/media_upload…

c362bc8

…_background_process

Merge branch 'gb/media_upload_progress_reattach' into gb/media_upload…

89fe3ec

…_background_process

mzorz changed the base branch from gb/media_upload_progress_reattach to gb/feature_upload_media_file January 28, 2019 14:21

marecar3 reviewed Jan 29, 2019

View reviewed changes

Merge branch 'gb/feature_upload_media_file' into gb/media_upload_back…

625e9a8

…ground_process

marecar3 reviewed Jan 29, 2019

View reviewed changes

mzorz added 8 commits January 29, 2019 15:48

removed unused context from parameter declaration

f797b98

put number in constant

6c5ec0a

replaced final int with class constant to make it easier to read

924725e

moved gutenberg header and class placeholders to constants

a321f5e

moved static methods to PostUtils

7472379

removed contentContainsGutenbergBlocks method from GutenbergEditorFra…

0fc2aa4

…gment as it was already moved to PostUtils before

removed unused import

2cdf852

removed unused import

cd3d599

marecar3 approved these changes Jan 29, 2019

View reviewed changes

mzorz merged commit 04479bb into gb/feature_upload_media_file Jan 29, 2019

mzorz deleted the gb/media_upload_background_process branch January 29, 2019 19:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gutenberg - async media upload Post background processing #9132

Gutenberg - async media upload Post background processing #9132

mzorz commented Jan 28, 2019 •

edited

Loading

mzorz Jan 28, 2019

marecar3 Jan 29, 2019

mzorz Jan 29, 2019

mzorz Jan 29, 2019

marecar3 Jan 29, 2019

mzorz Jan 29, 2019

marecar3 Jan 29, 2019

mzorz Jan 29, 2019

mzorz Jan 29, 2019

mzorz commented Jan 29, 2019

marecar3 left a comment

Gutenberg - async media upload Post background processing #9132

Gutenberg - async media upload Post background processing #9132

Conversation

mzorz commented Jan 28, 2019 • edited Loading

For further discussion

To test:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mzorz commented Jan 29, 2019

marecar3 left a comment

Choose a reason for hiding this comment

mzorz commented Jan 28, 2019 •

edited

Loading