Offline mode: Post conflict resolution overlay #20607

zwarm · 2024-04-07T20:59:58Z

Fixes #20164

This PR updates the dialogs for post version conflicts and autosave available. Instead of the small android dialog, they are now a full page overlay.

CC @osullivanchris - Any design suggestions or comments? Are we completely off base? Went off the iOS screens.

Before

Version Conflict	Autosave

After

Version Conflict Light	Version Conflict Dark

Version Conflict Light Selected	Version Conflict Dark Selected

Autosave Light	Autosave Dark

Autosave Light Selected	Autosave Dark Selected

Prepare for testing

Install the Jetpack and WordPress apps from this PR
Navigate to Me > Debug Settings
Enabled the sync_publishing flag and restart the app
Prepare a version conflict post and an autosave available post
Enable log collection to verify track events

Test Overlay (Run twice - once for Version Conflict & once for Autosave Available)

Tap on the post row that shows "Version Conflict" or "You've made unsaved changes to this post"
✅ Verify the correct resolution dialog is shown (see images above)
✅ Verify logs shows the tracking event 🔵 Tracked: resolve_autosave_conflict_screen_shown OR 🔵 Tracked: resolve_conflict_screen_shown
Select the checkbox next to Current Device
✅ Verify the Confirm button is enabled
Deselect the checkbox next to Current Device
✅ Verify the Confirm button is disabled
Select the checkbox next to Another Device
✅ Verify the Confirm button is enabled
Deselect the checkbox next to Another Device
✅ Verify the Confirm button is disabled
Tap on the "X" to close the overlay
✅ Verify the overlay is closed
✅ Verify logs shows the tracking event 🔵 Tracked: resolve_autosave_conflict_close_tapped OR 🔵 Tracked: resolve_conflict_close_tapped
Reopen the same post from the post list to relaunch the Version Conflict & once for Autosave Available overlay
Tap on the "Cancel" button to close the overlay
✅ Verify the overlay is closed
✅ Verify logs shows the tracking event 🔵 Tracked: resolve_autosave_conflict_cancel_tapped OR 🔵 Tracked: resolve_conflict_cancel_tapped
Reopen the same post from the post list to relaunch the Version Conflict & once for Autosave Available overlay
Select the Current Device and tap confirm
✅ Verify the overlay closes
✅ Verify logs shows the tracking event 🔵 Tracked: resolve_conflict_confirm_tapped, Properties: {"confirm_type":"local_version"} OR 🔵 Tracked: resolve_conflict_confirm_tapped, Properties: {"confirm_type":"remote_version"} OR 🔵 Tracked: resolve_autosave_conflict_confirm_tapped, Properties: {"confirm_type":"local_version"} OR 🔵 Tracked: resolve_autosave_conflict_confirm_tapped, Properties: {"confirm_type":"remote_version"}
✅ Verify the post was updated with the other device version

Regression Notes

Potential unintended areas of impact
The post conflicts are not resolved properly
What I did to test those areas of impact (or what existing automated tests I relied on)
Added PostResolutionOverlayViewModelTest and PostResolutionOverlayAnalyticsTrackerTest
What automated tests I added (or what prevented me from doing so)
Relied on existing instrumentation tests

PR Submission Checklist:

I have completed the Regression Notes.
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 (strike-out the not-applying and unnecessary ones):

dangermattic · 2024-04-07T21:00:29Z

	3 Warnings
⚠️	`strings.xml` files should only be updated on release branches, when the translations are downloaded by our automation.
⚠️	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️	This PR is assigned to the milestone 24.7. This milestone is due in less than 4 days. Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

wpmobilebot · 2024-04-07T21:16:43Z

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

	App Name	WordPress
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr20607-0f4f175
	Commit	`0f4f175`
	Direct Download	`wordpress-prototype-build-pr20607-0f4f175.apk`

Note: Google Login is not supported on these builds.

wpmobilebot · 2024-04-07T21:18:12Z

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

	App Name	Jetpack
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr20607-0f4f175
	Commit	`0f4f175`
	Direct Download	`jetpack-prototype-build-pr20607-0f4f175.apk`

Note: Google Login is not supported on these builds.

…nary action click

WordPress/src/main/java/org/wordpress/android/ui/posts/PostResolutionOverlayFragment.kt

WordPress/src/main/java/org/wordpress/android/ui/posts/PostResolutionOverlayListener.kt

WordPress/src/main/java/org/wordpress/android/ui/posts/PostListDialogHelper.kt

…ts as a result of tests

codecov · 2024-04-11T00:04:16Z

Codecov Report

Attention: Patch coverage is 37.37624% with 253 lines in your changes are missing coverage. Please review.

Project coverage is 40.38%. Comparing base (f833f0b) to head (0f4f175).
Report is 89 commits behind head on trunk.

Files	Patch %	Lines
...ordpress/android/ui/posts/PostResolutionOverlay.kt	0.00%	170 Missing ⚠️
...wordpress/android/ui/posts/PostListDialogHelper.kt	6.12%	46 Missing ⚠️
...android/ui/posts/PostResolutionOverlayViewModel.kt	83.33%	2 Missing and 14 partials ⚠️
...ordpress/android/ui/posts/PostListMainViewModel.kt	50.00%	7 Missing and 1 partial ⚠️
...s/android/ui/posts/PostResolutionOverlayUiState.kt	87.09%	3 Missing and 1 partial ⚠️
...ordpress/android/ui/uploads/PostUploadHandler.java	0.00%	3 Missing ⚠️
...ava/org/wordpress/android/util/DateUtilsWrapper.kt	0.00%	3 Missing ⚠️
...rg/wordpress/android/ui/posts/PostActionHandler.kt	50.00%	1 Missing ⚠️
...ress/android/ui/posts/editor/StorePostViewModel.kt	50.00%	1 Missing ⚠️
...org/wordpress/android/util/DateTimeUtilsWrapper.kt	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk   #20607      +/-   ##
==========================================
- Coverage   40.39%   40.38%   -0.02%     
==========================================
  Files        1474     1479       +5     
  Lines       67906    68276     +370     
  Branches    11226    11281      +55     
==========================================
+ Hits        27433    27570     +137     
- Misses      37994    38211     +217     
- Partials     2479     2495      +16

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

…change. Set isStarted and make post a property

pantstamp · 2024-04-12T10:08:10Z

Hey @zwarm ! Awesome work!

I reviewed the code and tested the app. I found the following:

Minor Issue: When the configuration is changed to landscape mode and the user scrolls down to view content, attempting to scroll back up can unintentionally dismiss the bottom sheet dialog due to overlapping gestures. Please refer to the recording for a demonstration. Perhaps we could consider creating a future PR to introduce a different layout for landscape mode that eliminates the need for scrolling. This is not a blocker for approving this PR.
Major Issue: During testing of the configuration changes, I observed that confirming the conflict resolution sometimes triggers multiple toasts. It's unclear whether this issue occurs solely on orientation change or if it also happens when we dismiss and reopen the dialog. I will investigate further, as I consider this a blocker. Please review the recording for more details.

sonarqubecloud · 2024-04-12T11:00:10Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
2.2% Duplication on New Code

See analysis details on SonarCloud

pantstamp · 2024-04-12T11:02:45Z

@zwarm I've pushed a fix for the major issue mentioned earlier. The problem was that we weren't checking whether the PostListMainViewModel had already been started. Consequently, during a configuration change, multiple instances of the PostListEventListener object were being created. These instances were triggering multiple events to the PostConflictResolver object, resulting in multiple toasts being displayed to the user.

I have tested the changes and they have resolved the issue, so I am approving the PR!

pantstamp

Great work! LGTM!

zwarm · 2024-04-12T11:28:26Z

@zwarm I've pushed a fix for the major issue mentioned earlier. The problem was that we weren't checking whether the PostListMainViewModel had already been started. Consequently, during a configuration change, multiple instances of the PostListEventListener object were being created. These instances were triggering multiple events to the PostConflictResolver object, resulting in multiple toasts being displayed to the user.

@pantstamp - this was a great find. I recall other reports of multiple toasts showing. Thanks for the fix. 🙇

zwarm · 2024-04-12T11:32:06Z

Minor Issue: When the configuration is changed to landscape mode and the user scrolls down to view content, attempting to scroll back up can unintentionally dismiss the bottom sheet dialog due to overlapping gestures.

Thanks for the video, I wonder if we should disallow swipe to close on landscape? Probably not, but we can try it.

osullivanchris · 2024-04-12T15:27:08Z

Hey I think its a good idea to use the Whats New format (also similar to Bloganuary) which it looks like you've done. I've just tidied up the type styles and stuff more appropriate for this case. I didn't have a modal and x button to hand but what you have there is fine.

Figma link eYeHXEMDbnFptE40xUqZ2T-fi-2551%3A6479

sentry-io · 2024-04-14T21:38:51Z

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

‼️ ApplicationNotResponding: ANR for at least 5000 ms. org.wordpress.android.ui.posts.PostsListActivit... View Issue

_{Did you find this useful? React with a 👍 or 👎}

zwarm · 2024-04-29T15:27:34Z

@osullivanchris - Thank you for the feedback, but somehow I missed the GH ping 🤦 . I'll add a new ticket to make this adjustments in an upcoming version.

zwarm added 6 commits April 7, 2024 16:49

Add wrapper around DateUtils for testability

e5de678

Initial versions of fragment and vm for PostResolutionOverlay

71ed026

Set up di modules for post resolution overlay

434d94d

Add wrapper around timestampFromIso8601Millis

f31e6c7

Add base strings for post resolution overlay

a2bf427

WIP - Show the post resolution overlay instead of the dialog.

f8339fe

zwarm added Post List [Type] Task labels Apr 7, 2024

zwarm self-assigned this Apr 7, 2024

zwarm added 8 commits April 8, 2024 08:18

Change save to confirm and use string resource for both buttons

fda28f0

Add a listener interface for post resolution

88786ae

Hooked in the initial version of the PostResolutionOverlayListener

27dc8a3

Hook in the initial version of the PostResolutionOverlayListener

c205244

WIP - Added self comments and todos

98cb540

Rename: PostConflictResolutionType to PostResolutionType

308bec0

Reduce the callbacks to a single function and handle downstream

19e5e87

Hook the dismiss actions (cancel, dismiss, and close) and the prelimi…

5220ce3

…nary action click

pantstamp reviewed Apr 9, 2024

View reviewed changes

WordPress/src/main/java/org/wordpress/android/ui/posts/PostResolutionOverlayFragment.kt Outdated Show resolved Hide resolved

pantstamp reviewed Apr 9, 2024

View reviewed changes

WordPress/src/main/java/org/wordpress/android/ui/posts/PostResolutionOverlayListener.kt Outdated Show resolved Hide resolved

pantstamp reviewed Apr 9, 2024

View reviewed changes

WordPress/src/main/java/org/wordpress/android/ui/posts/PostListDialogHelper.kt Outdated Show resolved Hide resolved

zwarm added 7 commits April 10, 2024 11:59

Make fun public so enabled can be accessed

68b3928

Pass the value of isSyncPublishingEnabled flag over to the dialogHelper

6619c9e

Add string resources for autosave overlay

8abc56f

Rename the listener function

d943074

Updates to reflect name changes and signature types

64a8ea3

Adjsut for name changes and handle onPostResolutionConfirmed

d41b26d

Adjust for renaming and implement autosave sync logic

08db006

zwarm added 3 commits April 10, 2024 16:49

Refactor: move composables out of fragment

a4e744d

Refactor: move UiState to its own file

c16d387

Add unit tests for PostResolutionOverlayViewModel and minor adjustmen…

e0bfc66

…ts as a result of tests

zwarm requested a review from AjeshRPai April 10, 2024 23:55

pantstamp and others added 9 commits April 11, 2024 12:54

refactor: rename SyncPublishingFeatureConfig

a69336c

Refactor: Allow PostResolutionListener to be null for detach purposes

f7dd712

Remove todo

37597dc

Check for fragment before creating

6cd3ad4

Annotate with JvmStatic

83e8880

Do not dismiss the dialog on an super dismiss to support orientation …

bb4997b

…change. Set isStarted and make post a property

Add post resolution tracking events

ee422b4

Add tracker for post resolution overlay

e71f620

Implement tracking for post resolution conflict overlay

8089791

zwarm marked this pull request as ready for review April 11, 2024 19:15

zwarm modified the milestones: 24.6 ❄️, 24.7 Apr 11, 2024

Fixes: PostListMainViewModel multiple starts

0f4f175

pantstamp approved these changes Apr 12, 2024

View reviewed changes

pantstamp merged commit 0f6f547 into trunk Apr 12, 2024
21 checks passed

pantstamp deleted the issue/20164-conflict-resolution-dialog branch April 12, 2024 11:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Offline mode: Post conflict resolution overlay #20607

Offline mode: Post conflict resolution overlay #20607

zwarm commented Apr 7, 2024 •

edited

Loading

dangermattic commented Apr 7, 2024 •

edited

Loading

wpmobilebot commented Apr 7, 2024 •

edited

Loading

wpmobilebot commented Apr 7, 2024 •

edited

Loading

codecov bot commented Apr 11, 2024 •

edited

Loading

pantstamp commented Apr 12, 2024

sonarqubecloud bot commented Apr 12, 2024

pantstamp commented Apr 12, 2024

pantstamp left a comment

zwarm commented Apr 12, 2024

zwarm commented Apr 12, 2024

osullivanchris commented Apr 12, 2024

sentry-io bot commented Apr 14, 2024

zwarm commented Apr 29, 2024

Offline mode: Post conflict resolution overlay #20607

Offline mode: Post conflict resolution overlay #20607

Conversation

zwarm commented Apr 7, 2024 • edited Loading

Prepare for testing

Test Overlay (Run twice - once for Version Conflict & once for Autosave Available)

Regression Notes

PR Submission Checklist:

Testing Checklist (strike-out the not-applying and unnecessary ones):

dangermattic commented Apr 7, 2024 • edited Loading

wpmobilebot commented Apr 7, 2024 • edited Loading

wpmobilebot commented Apr 7, 2024 • edited Loading

codecov bot commented Apr 11, 2024 • edited Loading

Codecov Report

pantstamp commented Apr 12, 2024

sonarqubecloud bot commented Apr 12, 2024

Quality Gate passed

pantstamp commented Apr 12, 2024

pantstamp left a comment

Choose a reason for hiding this comment

zwarm commented Apr 12, 2024

zwarm commented Apr 12, 2024

osullivanchris commented Apr 12, 2024

sentry-io bot commented Apr 14, 2024

Suspect Issues

zwarm commented Apr 29, 2024

zwarm commented Apr 7, 2024 •

edited

Loading

dangermattic commented Apr 7, 2024 •

edited

Loading

wpmobilebot commented Apr 7, 2024 •

edited

Loading

wpmobilebot commented Apr 7, 2024 •

edited

Loading

codecov bot commented Apr 11, 2024 •

edited

Loading