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

Gutenberg - async media upload Post background processing #9132

Merged
merged 16 commits into from
Jan 29, 2019
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -559,6 +559,9 @@ public void run() {
}

private void purgeMediaToPostAssociationsIfNotInPostAnymore() {
boolean useAztec = AppPrefs.isAztecEditorEnabled();
boolean useGutenberg = AppPrefs.isGutenbergEditorEnabled();

ArrayList<MediaModel> allMedia = new ArrayList<>();
allMedia.addAll(mUploadStore.getFailedMediaForPost(mPost));
allMedia.addAll(mUploadStore.getCompletedMediaForPost(mPost));
Expand All @@ -567,8 +570,16 @@ private void purgeMediaToPostAssociationsIfNotInPostAnymore() {
if (!allMedia.isEmpty()) {
HashSet<MediaModel> mediaToDeleteAssociationFor = new HashSet<>();
for (MediaModel media : allMedia) {
if (!AztecEditorFragment.isMediaInPostBody(this, mPost.getContent(), String.valueOf(media.getId()))) {
mediaToDeleteAssociationFor.add(media);
if (useAztec) {
if (!AztecEditorFragment.isMediaInPostBody(this,
mPost.getContent(), String.valueOf(media.getId()))) {
mediaToDeleteAssociationFor.add(media);
}
} else if (useGutenberg) {
if (!GutenbergEditorFragment.isMediaInPostBody(this,
mPost.getContent(), String.valueOf(media.getId()))) {
mediaToDeleteAssociationFor.add(media);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import org.wordpress.android.WordPress;
import org.wordpress.android.editor.AztecEditorFragment;
import org.wordpress.android.editor.EditorFragment;
import org.wordpress.android.editor.GutenbergEditorFragment;
import org.wordpress.android.fluxc.model.PostModel;
import org.wordpress.android.ui.media.services.MediaUploadReadyListener;
import org.wordpress.android.ui.prefs.AppPrefs;
Expand All @@ -17,8 +18,13 @@ public PostModel replaceMediaFileWithUrlInPost(@Nullable PostModel post, String
if (post != null) {
boolean showAztecEditor = AppPrefs.isAztecEditorEnabled();
boolean showNewEditor = AppPrefs.isVisualEditorEnabled();
boolean showGutenbergEditor = AppPrefs.isGutenbergEditorEnabled();

if (showAztecEditor) {
if (showGutenbergEditor) {
post.setContent(GutenbergEditorFragment
.replaceMediaFileWithUrl(WordPress.getContext(), post.getContent(),
localMediaId, mediaFile));
} else if (showAztecEditor) {
post.setContent(AztecEditorFragment.replaceMediaFileWithUrl(WordPress.getContext(), post.getContent(),
localMediaId, mediaFile));
} else if (showNewEditor) {
Expand All @@ -36,8 +42,11 @@ public PostModel markMediaUploadFailedInPost(@Nullable PostModel post, String lo
if (post != null) {
boolean showAztecEditor = AppPrefs.isAztecEditorEnabled();
boolean showNewEditor = AppPrefs.isVisualEditorEnabled();
boolean showGutenbergEditor = AppPrefs.isGutenbergEditorEnabled();

if (showAztecEditor) {
if (showGutenbergEditor) {
// TODO check if anything needs be done in Gutenberg
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This // TODO mark left here on purpose

} else if (showAztecEditor) {
post.setContent(AztecEditorFragment.markMediaFailed(WordPress.getContext(), post.getContent(),
localMediaId, mediaFile));
} else if (showNewEditor) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.wordpress.android.util.AppLog;
import org.wordpress.android.util.PermissionUtils;
import org.wordpress.android.util.ProfilingUtils;
import org.wordpress.android.util.StringUtils;
import org.wordpress.android.util.ToastUtils;
import org.wordpress.android.util.AppLog.T;
import org.wordpress.android.util.helpers.MediaFile;
Expand All @@ -39,6 +40,8 @@
import java.util.HashSet;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class GutenbergEditorFragment extends EditorFragmentAbstract implements
View.OnTouchListener,
Expand Down Expand Up @@ -364,6 +367,55 @@ public static boolean contentContainsGutenbergBlocks(String postContent) {
return (postContent != null && postContent.contains(GUTENBERG_BLOCK_START));
}

public static String replaceMediaFileWithUrl(Context context, @NonNull String postContent,
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed this change in 7472379

String localMediaId, MediaFile mediaFile) {
if (mediaFile != null) {
String remoteUrl = StringUtils.notNullStr(Utils.escapeQuotes(mediaFile.getFileURL()));
// 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);
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably we can put this <!-- wp:image {\"id\":%s} --> in some constant ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in a321f5e

String newImgBlockHeader = String.format("<!-- wp:image {\"id\":%s} -->", mediaFile.getMediaId());
postContent = postContent.replace(oldImgBlockHeader, newImgBlockHeader);

// replace class wp-image-id with serverMediaId, and url_holder with remoteUrl
String oldImgClass = String.format("class=\"wp-image-%s\"", localMediaId);
String newImgClass = String.format("class=\"wp-image-%s\"", mediaFile.getMediaId());
postContent = postContent.replace(oldImgClass, newImgClass);

// let's first find this occurrence and keep note of the position, as we need to replace the
// immediate `src` value before
int iStartOfWpImageClassAttribute = postContent.indexOf(newImgClass);
if (iStartOfWpImageClassAttribute != -1) {
// now search negatively, for the src attribute appearing right before
int iStartOfImgTag = postContent.lastIndexOf("<img", iStartOfWpImageClassAttribute);
if (iStartOfImgTag != -1) {
Pattern p = Pattern.compile("<img[^>]*src=[\\\"']([^\\\"^']*)");
Matcher m = p.matcher(postContent.substring(iStartOfImgTag));
if (m.find()) {
String src = m.group();
int startIndex = src.indexOf("src=") + 5;
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 addressed in 6c5ec0a

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

String srcTag = src.substring(startIndex, src.length());
// now replace the url
postContent = postContent.replace(srcTag, remoteUrl);
}
}
}
} else {
// TODO replace in GB Video block?
}
}
return postContent;
}

public static boolean isMediaInPostBody(Context context, @NonNull String postContent,
String localMediaId) {
// check if media is in Gutenberg Post
String imgBlockHeaderToSearchFor = String.format("<!-- wp:image {\"id\":%s} -->", localMediaId);
return postContent.indexOf(imgBlockHeaderToSearchFor) != -1;
}


/*
* TODO: REMOVE THIS ONCE AZTEC COMPLETELY REPLACES THE VISUAL EDITOR IN WPANDROID APP
*/
Expand Down