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

Add content logging to help debug a Story post crash #14191

Merged
merged 4 commits into from
Mar 4, 2021
Merged
Show file tree
Hide file tree
Changes from 3 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 @@ -3,14 +3,18 @@
import androidx.annotation.Nullable;

import org.wordpress.android.fluxc.model.PostModel;
import org.wordpress.android.fluxc.model.SiteModel;
import org.wordpress.android.util.helpers.MediaFile;

/**
* Callbacks - requests for editor capabilities to replace media once it's finished uploading
* and mark media failed if could not be uploaded
*/
public interface MediaUploadReadyListener {
// TODO: We're passing a SiteModel parameter here in order to debug a crash on SaveStoryGutenbergBlockUseCase.
// Once that's done, the parameter should be replaced with a site url String, like it was before.
// See: https://git.io/JqfhK
PostModel replaceMediaFileWithUrlInPost(@Nullable PostModel post, String localMediaId, MediaFile mediaFile,
String siteUrl);
@Nullable SiteModel site);
PostModel markMediaUploadFailedInPost(@Nullable PostModel post, String localMediaId, MediaFile mediaFile);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import com.wordpress.stories.compose.story.StoryFrameItem
import com.wordpress.stories.compose.story.StoryIndex
import org.wordpress.android.fluxc.model.PostModel
import org.wordpress.android.fluxc.model.SiteModel
import org.wordpress.android.fluxc.model.post.PostStatus.PRIVATE
import org.wordpress.android.ui.posts.EditPostRepository
import org.wordpress.android.ui.stories.prefs.StoriesPrefs
import org.wordpress.android.ui.stories.prefs.StoriesPrefs.TempId
Expand Down Expand Up @@ -93,6 +94,7 @@ class SaveStoryGutenbergBlockUseCase @Inject constructor(

fun findAllStoryBlocksInPostAndPerformOnEachMediaFilesJson(
renanferrari marked this conversation as resolved.
Show resolved Hide resolved
postModel: PostModel,
siteModel: SiteModel?,
listener: DoWithMediaFilesListener
) {
var content = postModel.content
Expand All @@ -115,16 +117,30 @@ class SaveStoryGutenbergBlockUseCase @Inject constructor(
content = listener.doWithMediaFilesJson(content, jsonString)
storyBlockStartIndex += HEADING_START.length
} catch (exception: StringIndexOutOfBoundsException) {
AppLog.e(EDITOR, "Error while parsing Story blocks: ${exception.message}")
if (shouldLogContent(siteModel, postModel)) {
AppLog.e(EDITOR, "HTML content of the post before the crash: ${postModel.content}")
}
crashLogging.reportException(exception, EDITOR.toString())
AppLog.e(EDITOR, exception.message)
}
}
}

postModel.setContent(content)
}

fun replaceLocalMediaIdsWithRemoteMediaIdsInPost(postModel: PostModel, mediaFile: MediaFile) {
// See: https://git.io/JqfhK
private fun shouldLogContent(siteModel: SiteModel?, postModel: PostModel) = siteModel != null &&
siteModel.isWPCom &&
!siteModel.isPrivate &&
postModel.password.isEmpty() &&
postModel.status != PRIVATE.toString()

fun replaceLocalMediaIdsWithRemoteMediaIdsInPost(
postModel: PostModel,
siteModel: SiteModel?,
mediaFile: MediaFile
) {
if (TextUtils.isEmpty(mediaFile.mediaId)) {
// if for any reason we couldn't obtain a remote mediaId, it's not worth spending time
// looking to replace anything in the Post. Skip processing for later in error handling.
Expand All @@ -133,6 +149,7 @@ class SaveStoryGutenbergBlockUseCase @Inject constructor(
val gson = Gson()
findAllStoryBlocksInPostAndPerformOnEachMediaFilesJson(
postModel,
siteModel,
object : DoWithMediaFilesListener {
override fun doWithMediaFilesJson(content: String, mediaFilesJsonString: String): String {
var processedContent = content
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.fluxc.model.PostModel;
import org.wordpress.android.fluxc.model.SiteModel;
import org.wordpress.android.ui.media.services.MediaUploadReadyListener;
import org.wordpress.android.ui.posts.PostUtils;
import org.wordpress.android.ui.prefs.AppPrefs;
Expand All @@ -23,15 +24,16 @@ public class MediaUploadReadyProcessor implements MediaUploadReadyListener {

@Override
public PostModel replaceMediaFileWithUrlInPost(@Nullable PostModel post, String localMediaId, MediaFile mediaFile,
String siteUrl) {
@Nullable SiteModel site) {
if (post != null) {
boolean showAztecEditor = AppPrefs.isAztecEditorEnabled();
boolean showGutenbergEditor = AppPrefs.isGutenbergEditorEnabled();

if (PostUtils.contentContainsWPStoryGutenbergBlocks(post.getContent())) {
mSaveStoryGutenbergBlockUseCase
.replaceLocalMediaIdsWithRemoteMediaIdsInPost(post, mediaFile);
.replaceLocalMediaIdsWithRemoteMediaIdsInPost(post, site, mediaFile);
} else if (showGutenbergEditor && PostUtils.contentContainsGutenbergBlocks(post.getContent())) {
String siteUrl = site != null ? site.getUrl() : "";
post.setContent(
PostUtils.replaceMediaFileWithUrlInGutenbergPost(post.getContent(), localMediaId, mediaFile,
siteUrl));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -624,7 +624,7 @@ private static synchronized PostModel updatePostWithMediaUrl(PostModel post, Med

// actually replace the media ID with the media uri
processor.replaceMediaFileWithUrlInPost(post, String.valueOf(media.getId()),
FluxCUtils.mediaFileFromMediaModel(media), site.getUrl());
FluxCUtils.mediaFileFromMediaModel(media), site);

// we changed the post, so let’s mark this down
if (!post.isLocalDraft()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import org.mockito.junit.MockitoJUnitRunner
import org.wordpress.android.BaseUnitTest
import org.wordpress.android.TEST_DISPATCHER
import org.wordpress.android.fluxc.model.PostModel
import org.wordpress.android.fluxc.model.SiteModel
import org.wordpress.android.fluxc.store.PostStore
import org.wordpress.android.ui.posts.EditPostRepository
import org.wordpress.android.ui.posts.mediauploadcompletionprocessors.TestContent
Expand Down Expand Up @@ -140,10 +141,12 @@ class SaveStoryGutenbergBlockUseCaseTest : BaseUnitTest() {
val mediaFile = getMediaFile(1)
val postModel = PostModel()
postModel.setContent(BLOCK_WITH_NON_EMPTY_MEDIA_FILES)
val siteModel = SiteModel()

// When
saveStoryGutenbergBlockUseCase.replaceLocalMediaIdsWithRemoteMediaIdsInPost(
postModel,
siteModel,
mediaFile
)

Expand Down Expand Up @@ -178,9 +181,10 @@ class SaveStoryGutenbergBlockUseCaseTest : BaseUnitTest() {
whenever(mediaFile.fileURL).thenReturn(TestContent.remoteImageUrl)
val postModel = PostModel()
postModel.setContent(TestContent.storyBlockWithLocalIdsAndUrls)
val siteModel = SiteModel()

// act
saveStoryGutenbergBlockUseCase.replaceLocalMediaIdsWithRemoteMediaIdsInPost(postModel, mediaFile)
saveStoryGutenbergBlockUseCase.replaceLocalMediaIdsWithRemoteMediaIdsInPost(postModel, siteModel, mediaFile)

// assert
Assertions.assertThat(postModel.content).isEqualTo(TestContent.storyBlockWithFirstRemoteIdsAndUrlsReplaced)
Expand Down