Today Widget - Fetch remote stats - #15391
Merged
Gio2018 merged 7 commits intoNov 29, 2020
Merged
Conversation
added 6 commits
November 20, 2020 10:42
- Also using TimeZone instead of TimeZoneName as the time zone property in HomeWidgetData
…15347-today-widget-remote-stats
…press-mobile/WordPress-iOS into feature/15347-today-widget-remote-stats
…y widget - also add logic in WordPressHomeWidgetToday to only fetch remote stats if the cached values exist and are older than a given constant time
- store newly fetched data to local cache in getTimelineData, WordPressHomeWidgetToday - fixed some naming in StatsInsightsStore, HomeWidgetTodayData, HomeWidgetCache
|
You can trigger an installable build for these changes by visiting CircleCI here. |
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
Gio2018
marked this pull request as draft
November 25, 2020 18:37
Contributor
Author
|
@frosty @diegoreymendez please hold on this one, I have found a bug that needs fixing first. Sorry for the confusion! |
Gio2018
marked this pull request as ready for review
November 25, 2020 21:06
Contributor
Author
|
@diegoreymendez @frosty , this PR is back in the game!! |
diegoreymendez
approved these changes
Nov 27, 2020
diegoreymendez
left a comment
Contributor
There was a problem hiding this comment.
This works really well for me. I haven't found anything to comment on the code, honestly... it looks rather clean to me and I really like that we're using the service.
Contributor
Author
|
Thanks for reviewing @diegoreymendez @frosty !! |
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #15347
Notes:
WordPressHomeWidgetTodaythere are two time constants:refreshInterval: this is the refresh interval of the widget itself, set to 1 hourminElapsedTimeToRefresh: this is the minimum elapsed time to request a refresh for a cached item. If, at the moment a cached element is read, it's older than this time, it will be re-fetched, otherwise it will be kept. This takes into account that a cached element can happen in several points. For example, if the app refreshes the cache after 58 minutes from the previous widget refresh, there is no need for the widget to make another network call when the hour passes.To test:
Pre-requisites
andminElapsedTimeToRefresh`.As an example, you could set:
refreshInterval = 5(as far as I have seen, iOS would override smaller values anyway.)minElapsedTimeToRefresh = 1Testing
refreshInterval(keep in mind that iOS will decide when exactly to refresh the widget anyway), and make sure that the stats get updated in the widgetPR submission checklist:
RELEASE-NOTES.txtif necessary.