Offline Mode: Sync Engine by kean · Pull Request #22899 · wordpress-mobile/WordPress-iOS

kean · 2024-03-26T00:10:27Z

This PR introduces a new sync engine for draft posts (see PostCoordinator.setNeedsSync(for:) and the related methods).

There are a lot of additions, but it's mostly unit tests.

The unit test target has some unrelated failures due to the WordPressKit update. I'll need to merge trunk in order to fix it and trunk needs this change to be merged #22900 first.

Architecture

Data

When you open an editor, it now always creates a new revision. For example:

// Before open
R0 → R1 (sync-needed)

// Editor opened
R0 → R1 (sync-needed) → R2

The editor works with R2. The previous revisions are immutable.

The app not longer uses the existing remoteStatus property. Instead, it uses a simple isSyncNeeded boolean to determine which revisions to sync.

This amount of data is just enough to implement nearly any sync algorithm and to also fix issues like #21940.

Sync

PostCoordinator periodically syncs the revisions that have a isSyncNeeded flag set to true. It uses the new PostRepository.sync method introduces in the previous PRs. It creates a diff between the original and the most recent revision that needs sync, and uploads the changes using partial updates.

Known Issues

If media fails with a .notConnectedToInternet, the app doesn't display the "offline changes" label and doesn't perform quick retries

Testing

The changes were tested only using REST APIs and only using a simplest scenarios where you edit a post and then tap "back" to save or discard changes. The remaining flows will be updated in the future PRs.

Discard changes (new draft)

Create a new draft
Verify that if you tap back, the post is deleted without a confirmation dialog
Verify that if you make any changes to the post and tap "back", it shows a confirmation dialog where you can discard or save the draft

Discard changes (existing draft)

Edit an existing draft
Verify that if you tap back, the editor closes right away
Verify that if you make any changes to the post and tap "back", it shows a confirmation dialog where you can discard or save the changes

Discard changes (fix #21940)

Follow the steps from the issue #21940 and verify that it no longer reproduces.

Create new post with image

Create a new draft
Add an image
Tap back and tap "Save Draft" before the image gets fully uploaded
Verify that the image get uploaded
Verify that the post is created on the server with an image
Verify that no alerts or snackbars are shown at any point during sync

Creating and updating unsynced a draft

Set a breakpoint on /post/new
Create and save a draft
Verify that it hits the breakpoint
Edit the draft (e.g. change the tilte) and save the changes
Execute the original request
Verify that the post was created
Verify that app sends a follow-up request to save the changes on the server using partial updates
Verify that the post got updated and these is only one post on the server

Offline mode

Precondition: you are offline.

Create and save a new draft
Verify that the cell shows "Offline changes"
Verify that you can open the post for editing and make more changes
Enable connectivity
Verify that the app uploads the post nearly instantaneously (you can look for "connection is reachable – retrying now" in the logs)

Offline mode and restart

Create and save a new draft
Verify that the cell shows "Offline changes"
Terminate the app
Enable connectivity
Re-open the app
Verify that the post gets uploaded

Sync failure and retry

Precondition: request fails for any reason other than "404"

Create a new or update an existing draft
Save draft
Verify that the post gets synced with a progressively long delay (5, 7.5, 11.25...) until it reaches the maximum of 60 seconds

Discarded images

Create a new draft
Create a revision with a new image
Save the changes by tapping "Back" and "Save Changes"
Create another revision and delete the image
Save the changes
Enable connectity
Verify that the image does no get uploaded and does not block the app from uploading the changes

Regression Notes

Potential unintended areas of impact: Draft Posts
What I did to test those areas of impact (or what existing automated tests I relied on): manual and automated integration tests
What automated tests I added (or what prevented me from doing so): yes

PR submission checklist:

I have completed the Regression Notes.
I have considered adding unit tests for my changes.
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.

Testing checklist:

dangermattic · 2024-03-26T00:11:05Z

	2 Warnings
⚠️	View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.
⚠️	This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

wpmobilebot · 2024-03-26T00:19:55Z

📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.

	App Name	WordPress Alpha
	Configuration	Release-Alpha
	Build Number	pr22899-15f930e
	Version	24.5
	Bundle ID	org.wordpress.alpha
	Commit	`15f930e`
	App Center Build	WPiOS - One-Offs #9339

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

wpmobilebot · 2024-03-26T00:20:41Z

📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.

	App Name	Jetpack Alpha
	Configuration	Release-Alpha
	Build Number	pr22899-15f930e
	Version	24.5
	Bundle ID	com.jetpack.alpha
	Commit	`15f930e`
	App Center Build	jetpack-installable-builds #8383

Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

kean · 2024-03-26T18:10:59Z

WordPress/Classes/Models/AbstractPost.swift

+    }
+
+    // TODO: Replace with a new flag
+    @objc var isSyncNeeded: Bool {


I'm planning to add a new flag to the database but only after we merge the changes in trunk and after 24.6 is cut. We'll be also removing some data.

kean · 2024-03-26T18:15:06Z

WordPress/Classes/Models/Post.swift

-    // MARK: - Content Preview
-
-    fileprivate func buildContentPreview() {
-        if let excerpt = mt_excerpt, excerpt.count > 0 {


This was buggy and not effective as it would constantly reset.

kean · 2024-03-26T18:58:36Z

WordPress/Classes/Services/PostHelper.m

+}
+
+ (void)updatePost:(AbstractPost *)post withRemotePost:(RemotePost *)remotePost inContext:(NSManagedObjectContext *)managedObjectContext overwrite:(BOOL)overwrite {
+    if ([RemoteFeature enabled:RemoteFeatureFlagSyncPublishing] && (post.revision != nil && !overwrite)) {


If there are local revision, only sync (and trash in the future) should be able to update the post with the remote changes. I'm planning to simplify this code a bit later when we remove the feature flag.

kean · 2024-03-26T18:59:49Z

WordPress/Classes/ViewRelated/Post/PostEditor+Publish.swift

+        }
+    }
+
+    func performEditorAction(_ action: PostEditorAction, analyticsStat: WPAnalyticsStat?) {


This change should probably not be part of this PR, but it's too late too change. I suggest we roll forward and continue working on it in the future PRs. It's in the branch ,so it's safe.

kean · 2024-03-26T19:05:04Z

WordPress/Classes/ViewRelated/Post/PostEditor+Publish.swift

+
+        assert(post.latest() == post, "Must be opened with the latest verison of the post")
+
+        if !(post.isRevision() && !post.isSyncNeeded) {


This check is needed only for a scenario where the app crashes during editor – it will be covered later. I plan to re-open the editor automatically to make sure the user either commits of discard the revision.

kean · 2024-03-26T19:05:46Z

WordPress/Classes/ViewRelated/Post/PostListEditorPresenter.swift

-            return
+        if RemoteFeatureFlag.syncPublishing.enabled() {
+            // Return early if a post is still uploading when the editor's requested.
+            guard !PostCoordinator.shared.isUpdating(post) else {


Not great naming – will probably rework later once we start tearing the old code out.

kean · 2024-03-26T19:07:09Z

WordPress/Classes/ViewRelated/Post/PostListViewController.swift

        let updatedIndexPaths = (tableView.indexPathsForVisibleRows ?? []).filter {
            let post = fetchResultsController.object(at: $0)
-            return updatedObjects.contains(post)
+            return updatedObjects.contains(post) || updatedObjects.contains(post.original())


The height no longer changes, so we should now move the notification observer to the cell itself, which will be safer

momo-ozawa

If you save a new draft, then reopen the editor, the "Update" button is enabled from the get go. It should only be enabled if the user makes any changes.

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-03-28.at.18.41.30.mp4

When testing offline mode, the app crashes when the wifi reconnects. I waited for the "Offline changes" to appear in the post list cell, then reconnected the wifi, at which point the app crashed

kean · 2024-03-29T13:18:07Z

If you save a new draft, then reopen the editor, the "Update" button is enabled from the get go. It should only be enabled if the user makes any changes.

It's been fixed in the newer PR where it now displays a "Publish" button #22913

When testing offline mode, the app crashes when the wifi reconnects. I waited for the "Offline changes" to appear in the post list cell, then reconnected the wifi, at which point the app crashed

Good catch. I only tested it once on a real device and it somehow worked. I think it was because I didn't have a debugger attached, so it didn't trigger a Core Data concurrency error. Fixed.

kean · 2024-04-01T21:35:35Z

WordPress/Classes/ViewRelated/Post/PostEditor+Publish.swift

-                }
-                self.postEditorStateContext.updated(isBeingPublished: false)
-                SVProgressHUD.dismiss()
+        PostCoordinator.shared.save(post, defaultFailureNotice: uploadFailureNotice(action: action)) { [weak self] result in


Reverting this change as we are now handling the feature flag on a higher level – at the button tap.

momo-ozawa

Thanks for addressing the issues from my previous review! Reconnecting doesn't crash anymore 👍

kean added [Type] Task Offline Mode (Sync Issues) labels Mar 26, 2024

kean force-pushed the task/sync-engine branch from daf59db to 06ad299 Compare March 26, 2024 00:11

kean changed the base branch from feature/offline-mode-milesone-2 to task/simplify-post-settings March 26, 2024 01:43

kean commented Mar 26, 2024

View reviewed changes

kean force-pushed the task/sync-engine branch from 754e981 to d48584d Compare March 26, 2024 18:26

kean changed the base branch from task/simplify-post-settings to task/update-move-to-draft March 26, 2024 18:54

kean commented Mar 26, 2024

View reviewed changes

kean marked this pull request as ready for review March 26, 2024 19:08

kean requested a review from momo-ozawa March 26, 2024 19:08

kean added this to the Pending milestone Mar 26, 2024

wordpress-mobile deleted a comment Mar 26, 2024

kean marked this pull request as draft March 28, 2024 15:38

kean marked this pull request as ready for review March 28, 2024 15:50

momo-ozawa reviewed Mar 28, 2024

View reviewed changes

kean force-pushed the task/update-move-to-draft branch from 051bebf to 8078ab5 Compare April 1, 2024 17:32

kean requested a review from a team as a code owner April 1, 2024 17:32

kean force-pushed the task/sync-engine branch from bd49308 to 730f856 Compare April 1, 2024 17:49

kean force-pushed the task/update-move-to-draft branch from 8078ab5 to e11aa84 Compare April 1, 2024 21:28

kean force-pushed the task/sync-engine branch from 730f856 to f64287b Compare April 1, 2024 21:33

kean commented Apr 1, 2024

View reviewed changes

kean force-pushed the task/sync-engine branch from f64287b to f41c02e Compare April 1, 2024 21:36

kean requested a review from momo-ozawa April 1, 2024 21:38

Add new sync engine for draft posts

15f930e

kean force-pushed the task/sync-engine branch from f41c02e to 15f930e Compare April 1, 2024 22:21

momo-ozawa approved these changes Apr 2, 2024

View reviewed changes

Base automatically changed from task/update-move-to-draft to feature/offline-mode-milesone-2 April 3, 2024 00:14

kean merged commit 02ccb5e into feature/offline-mode-milesone-2 Apr 3, 2024

kean deleted the task/sync-engine branch April 3, 2024 00:14

momo-ozawa modified the milestones: Pending, 24.7, 24.8 Apr 15, 2024


		assert(post.latest() == post, "Must be opened with the latest verison of the post")

		if !(post.isRevision() && !post.isSyncNeeded) {

Conversation

kean commented Mar 26, 2024 • edited by momo-ozawa Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Architecture

Data

Sync

Known Issues

Testing

Discard changes (new draft)

Discard changes (existing draft)

Discard changes (fix #21940)

Create new post with image

Creating and updating unsynced a draft

Offline mode

Offline mode and restart

Sync failure and retry

Discarded images

Regression Notes

Uh oh!

dangermattic commented Mar 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Mar 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Mar 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

momo-ozawa left a comment

Choose a reason for hiding this comment

Uh oh!

kean commented Mar 29, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

momo-ozawa left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kean commented Mar 26, 2024 •

edited by momo-ozawa

Loading

dangermattic commented Mar 26, 2024 •

edited

Loading

wpmobilebot commented Mar 26, 2024 •

edited

Loading

wpmobilebot commented Mar 26, 2024 •

edited

Loading