Create a standalone TimeLineProvider and TimelineEntry for the lock screen by tzef · Pull Request #20368 · wordpress-mobile/WordPress-iOS

tzef · 2023-03-20T11:56:05Z

This is the PR for the refactoring of the timeline provider for lock screen widgets.

Add new feature flag for lock screen widgets in Jetpack #20309 (✅ Approved)
Implement lock screen widgets UI #20312 (✅ Approved)
Add dynamic type support #20342 (TBD)
Add localizable string for "Views Today" title #20353 (✅ Approved)
[BugFix] Fix widget of home screen for WordPress disappeared #20371 (✅ Approved)
Extract content logic to config from lock screen widget #20317 (✅ Approved)
Create a standalone TimeLineProvider and TimelineEntry for the lock screen #20368 (✅ Approved) 👈 you're here!
Implement view for logged-out, no-site, and no-data status #20399 (✅ Approved)
Notify to reload the timelines for lock screen widget #20405 (✅ Approved)
(Confirm the data display on UI correctly in this phase)
Implement event tracking for the new lock screen widget #20422 (✅ Approved)
Disable lockScreenWidget feature flag #20427 (In Reviewing)

Description

While it is working to reuse the home screen timeline provider and entry for the lock screen, there are some differences in logic. For instance, the disable check and status are redundant for the lock screen widget.

Creating a timeline provider and entry specific to the lock screen to separate the concerns, which should be helpful for improving understandability and maintainability. Here are the steps I took to achieve this:

Add LockScreenStatsWidgetData to define the properties the lock screen widgets need.
Add LockScreenSiteListProvider to handle the LockScreenStatsWidgetEntry timeline.
Extract the widget load to WidgetDataReader for reusability and testability.
Expand the result type in StatsWidgetsService for sharing the fetchStats with two providers.
Update the view provider and view to be based on the lock screen widget, removing unnecessary type handling.

Testing Instructions

This refractor did not introduce any new features. The following tests are for regression testing purposes.

In WordPress target

Won't impact existing widgets

Added today, all-time, and this-week home screen widgets
Upgrade the app
Existing widgets still work well

New widget can be added

Upgrade the app
WordPress in the widget selection page in the home screen editor
Able to add today, all-time, and this-week widgets and display the correct stats data

Status switching

Display disable message when fwf off
Display no-site message when login with a new account without setup websites
Display logged out message when logout the account

In Jetpack target

Won't impact home screen widgets

As same as WordPress testing instructions

Won't impact lock screen widgets (refresh mechanism not implemented yet)

Existing lock screen widgets work well after upgrading the app
Able to add today views widget to the lock screen and display the correct stats data

Status switching

Jetpack others status not implemented yet, status switching testing would in next PR

Regression Notes

Potential unintended areas of impact
Home screen widgets
What I did to test those areas of impact (or what existing automated tests I relied on)
Refer to the above testing Instructions
What automated tests I added (or what prevented me from doing so)
WidgetDataReaderTests for testing status check

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.

staskus

I know this is still a draft, posting interesting questions.

Let me know when it's ready for review. Thanks!

Update stats widgets service result type to consist of home widget and lock screen widget for reusable

Update associated type widget data to conform `LockScreenStatsWidgetData` more Update model mapper and view provider to based on lock screen widget data Remove unnecessary `disabled` status from `LockScreenStatsWidgetsView` Remove unnecessary `widgetKind` parameter from `LockScreenStatsWidget` Remove unused statsURL converter from the view model mapper Add home widget data conform lock screen widget data extension to WordPress target for unitTest

…line provider

…data reader with status callback

Extract cache reading to `HomeWidgetDataFileReader` for mocking the testing object Extract app configuration is jetpack to parameter for mocking the config

peril-wordpress-mobile · 2023-03-23T10:56:09Z

	Warnings
⚠️	PR is not assigned to a milestone.

Generated by 🚫 dangerJS

tzef

@staskus , as this PR changed existing provider quite much, I will do more testings, and you can take the review in the meanwhile, thanks 🙏

tzef · 2023-03-23T11:03:24Z

@@ -0,0 +1,12 @@
+import Foundation
+
+final class HomeWidgetDataFileReader: WidgetDataCacheReader {


Extracting the logic for testing the widget data reader

staskus · 2023-03-23T12:48:40Z

@tzef thanks for the work! 👍

I reviewed the extracted timeline code and submitted my comments so you could check them as early as possible.

I'll test the solution and also check the tests you've written.

staskus · 2023-03-23T14:18:02Z

Tested.

✅ WordPress/Jetpack home widget regression tests passed. All the different states appear correctly.
✅ New widgets show the data. As you mentioned, data refresh is still missing.

tzef · 2023-03-23T14:52:25Z

Updated the code to use Result and a custom enum for exception status. This makes the caller side much more straightforward than using callbacks. Thank you for the great suggestions. 👍

staskus

@tzef thanks for the changes! Looks good. The success case now is very explicit and clear.

I'm rerunning the CI and merging if all is good.

staskus · 2023-03-24T07:00:10Z

@tzef merged

* Add new feature flag for lock screen widgets in Jetpack (#20309) * Add new feature flag for widget in lock screen * Add new widget kind to widget configuration * Add empty lock screen widget * Add JETPACK_STATS_WIDGETS compilation condition To prevent lock screen widgets from displaying on WordPress target * Move LockScreenStatsWidgetsView to views folder group * Revert compilation condition, change to isJetpack of AppCondiguration * Implement lock screen widgets UI (#20312) * Extract statsUrl from private to extension for reusable with lock screen widget * Implement single stat view for today views widget Add view provider for handling building view for site selected status Add ViewModelMapper for converting viewModel from home widget data * Update LockScreenStatsWidgetsView body to get the view from view provider * Add mapper test to verify the viewModel converting from widget data correctly Add mapper and viewModel to WordPress target because app extension is not able to run unitTest * Fix getStatsURL typo * Add localizable string for "Views Today" title (#20353) * Add views in today localizable string for footer label * Remove dateRange and footer, only title is enough for existing feature * Fix home screen widget for WordPress target disappeared. (#20371) The root cause is the WidgetBundleBuilder body doesn't allow control flow statements, move it to supportedFamilies with feature flag to keep all control logic in the same place * Extract content logic to config from lock screen widget (#20317) * Update title of view provider from hardcoded to pass from config * Add lock screen stats widget config and implementation to include required information * Pass config to LockScreenStatsWidget Replaced the hardcoded data with config properties * Add associated type view for other status and TODO comments * Extract app configuration and feature flag check into config * Create a standalone TimeLineProvider and TimelineEntry for the lock screen (#20368) * Add widget data and timeline entry for lock screen to separate concerns * Add lock screen time line provider for lock screen stats widget entry Update stats widgets service result type to consist of home widget and lock screen widget for reusable * Update to use LockScreenSiteListProvider in LockScreenStatsWidget Update associated type widget data to conform `LockScreenStatsWidgetData` more Update model mapper and view provider to based on lock screen widget data Remove unnecessary `disabled` status from `LockScreenStatsWidgetsView` Remove unnecessary `widgetKind` parameter from `LockScreenStatsWidget` Remove unused statsURL converter from the view model mapper Add home widget data conform lock screen widget data extension to WordPress target for unitTest * Extract widget load data to loader for sharing with lock screen time line provider * Update widget reader to extract the cache read from getSnapshot * Extract noData, noSite, loggedOut, disabled status checking logic to data reader with status callback * Add unitTest for widget data reader Extract cache reading to `HomeWidgetDataFileReader` for mocking the testing object Extract app configuration is jetpack to parameter for mocking the config * Remove multiple space lines * Update multiple callback to Result and enum Error * Implement view for logged-out, no-site, and no-data status (#20399) * Add unconfigured view for lockscreen widget * Add unconfigured view model to view provider and model mapper * Add unconfigured view model unit test * Add background to unconfigured view align with success status widget appearance * Remove todo comments * Add reload timelines for lock screen widget (#20405) * Implement event tracking for the new lock screen widget (#20422) * Add event key to configuration and pass to trackers in lockscreen widget Expand tracks for lockscreen stats entry Add rectangular widgets count and widgets update of the new widget * Add lockscreen_widget source to deeplink tracked Update statsUrl to widgetUrl for separating the url from homescreen widgets * Update unitTests * Remove separate widget url, move widget source from widget data extension Add widget source type, assign the type in widgetURL in the view * Add widget url source test to verify the converting with DeepLinkSource * Disable the feature flag (#20427) --------- Co-authored-by: Beemo Lee <tzef8220@gmail.com>

tzef marked this pull request as draft March 20, 2023 11:56

tzef force-pushed the feature/beemotrial/lockscreen-widgets-entry branch 3 times, most recently from 07778b5 to 912477e Compare March 22, 2023 04:39

staskus reviewed Mar 22, 2023

View reviewed changes

Comment thread WordPress/WordPressStatsWidgets/LockScreenWidgets/LockScreenSiteListProvider.swift

Comment thread WordPress/WordPressStatsWidgets/Helpers/WidgetDataReader.swift

tzef added 6 commits March 23, 2023 12:56

Add widget data and timeline entry for lock screen to separate concerns

d621226

Add lock screen time line provider for lock screen stats widget entry

14e7e41

Update stats widgets service result type to consist of home widget and lock screen widget for reusable

Extract widget load data to loader for sharing with lock screen time …

8e2c986

…line provider

Update widget reader to extract the cache read from getSnapshot

ffb7380

Extract noData, noSite, loggedOut, disabled status checking logic to …

81da0ae

…data reader with status callback

tzef force-pushed the feature/beemotrial/lockscreen-widgets-entry branch 2 times, most recently from ee7659a to 3fda761 Compare March 23, 2023 09:34

Add unitTest for widget data reader

2c4d665

Extract cache reading to `HomeWidgetDataFileReader` for mocking the testing object Extract app configuration is jetpack to parameter for mocking the config

tzef force-pushed the feature/beemotrial/lockscreen-widgets-entry branch from 3fda761 to 2c4d665 Compare March 23, 2023 09:48

Remove multiple space lines

4a2d013

tzef commented Mar 23, 2023

View reviewed changes

tzef marked this pull request as ready for review March 23, 2023 11:05

staskus reviewed Mar 23, 2023

View reviewed changes

staskus added Stats iOS 16 Widgets labels Mar 23, 2023

Update multiple callback to Result and enum Error

89cb3d7

staskus approved these changes Mar 23, 2023

View reviewed changes

tzef mentioned this pull request Mar 24, 2023

Implement view for logged-out, no-site, and no-data status #20399

Merged

4 tasks

staskus merged commit 0e87db2 into wordpress-mobile:feature/19411-add-lock-screen-stats-widgets Mar 24, 2023

staskus mentioned this pull request Mar 30, 2023

iOS 16 Lock Screen Widgets: Today Views Widget #20435

Merged

4 tasks

		@@ -0,0 +1,12 @@
		import Foundation

		final class HomeWidgetDataFileReader: WidgetDataCacheReader {

Uh oh!

Conversation

tzef commented Mar 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing Instructions

In WordPress target

Won't impact existing widgets

New widget can be added

Status switching

In Jetpack target

Won't impact home screen widgets

Won't impact lock screen widgets (refresh mechanism not implemented yet)

Status switching

Regression Notes

Uh oh!

staskus left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

peril-wordpress-mobile Bot commented Mar 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tzef left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tzef Mar 23, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

staskus commented Mar 23, 2023

Uh oh!

staskus commented Mar 23, 2023

Uh oh!

tzef commented Mar 23, 2023

Uh oh!

staskus left a comment

Choose a reason for hiding this comment

Uh oh!

staskus commented Mar 24, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tzef commented Mar 20, 2023 •

edited

Loading

peril-wordpress-mobile Bot commented Mar 23, 2023 •

edited

Loading