Skip to content

[Stats Refresh] Period Overview card: populate with real data#11485

Merged
ScoutHarris merged 2 commits intodevelopfrom
feature/11205-period_overview_card_data
Apr 18, 2019
Merged

[Stats Refresh] Period Overview card: populate with real data#11485
ScoutHarris merged 2 commits intodevelopfrom
feature/11205-period_overview_card_data

Conversation

@ScoutHarris
Copy link
Contributor

Fixes #11205

This uses real summary data for the Period Overview card. To note, the chart still uses stub data.

NOTE: As noted on #11342 (comment), fetching summary data can take a really long time, especially with longer Periods. Patience is a virtue.

To test:

  • Go to Stats > Period.
  • Verify the numbers populate real data.
  • Verify the data updates when different periods are selected (D, W, M, Y).
  • Verify the data updates when different dates are selected via the date bar.

overview_data

@ScoutHarris ScoutHarris added this to the 12.3 milestone Apr 17, 2019
@ScoutHarris ScoutHarris requested a review from jklausa April 17, 2019 20:21
@ScoutHarris ScoutHarris self-assigned this Apr 17, 2019
@ScoutHarris
Copy link
Contributor Author

@jklausa - could you just verify I am fetching summary data correctly? I simply added back what you removed with 5869285. Thanks!

@ScoutHarris ScoutHarris mentioned this pull request Apr 17, 2019
45 tasks
@jklausa
Copy link
Contributor

jklausa commented Apr 18, 2019

argh, I completely forgot about this.

I took a quick peek to see whether the slowness is my fault, or the actual network taking a long time, using a ages-old tradition of NSLog debugging:

diff --git a/WordPressKit/StatsServiceRemoteV2.swift b/WordPressKit/StatsServiceRemoteV2.swift
index e0e9409..da9b486 100644
--- a/WordPressKit/StatsServiceRemoteV2.swift
+++ b/WordPressKit/StatsServiceRemoteV2.swift
@@ -78,6 +78,8 @@ public class StatsServiceRemoteV2: ServiceRemoteWordPressComREST {
             return val1
         }
 
+
+        NSLog("dispatched network call for \(period.stringValue) ending on \(endingOn)")
         wordPressComRestApi.GET(path, parameters: properties, success: { (response, _) in
             guard
                 let jsonResponse = response as? [String: AnyObject],
@@ -88,6 +90,10 @@ public class StatsServiceRemoteV2: ServiceRemoteWordPressComREST {
                     return
             }
 
+
+                NSLog("received summary data")
+
+
             let periodString = jsonResponse["period"] as? String
             let parsedPeriod = periodString.flatMap { StatsPeriodUnit(string: $0) } ?? period
             // some responses omit this field!  not a reason to fail a whole request parsing though.
@@ -97,10 +103,12 @@ public class StatsServiceRemoteV2: ServiceRemoteWordPressComREST {
                                               period: parsedPeriod,
                                               jsonDictionary: jsonResponse)
                 else {
+                    NSLog("failled mapping, returning nil")
                     completion(nil, ResponseError.decodingFailure)
                     return
             }
 
+            NSLog("finished parsing summary")
             completion(timestats, nil)
         }, failure: { (error, _) in
             completion(nil, error)

and as I suspected, parsing of this response is rather quick (on the order of magnitude of ~300 ms), but the actual request takes almost a minute (!!!) for getting a year's worth of data — it seems to grow linearly with the amount of views.

here are the logs:

fetching for day — about a second

2019-04-18 13:10:53.950101+0200 WordPress[97705:22645475] dispatched network call for day ending on 2019-04-18 11:10:53 +0000
2019-04-18 13:10:55.172330+0200 WordPress[97705:22645475] received summary data
2019-04-18 13:10:55.173953+0200 WordPress[97705:22645475] finished parsing summary

for a week — 6 seconds

2019-04-18 13:11:04.987464+0200 WordPress[97705:22645475] dispatched network call for week ending on 2019-04-18 11:11:04 +0000
2019-04-18 13:11:10.604702+0200 WordPress[97705:22645475] received summary data
2019-04-18 13:11:10.605941+0200 WordPress[97705:22645475] finished parsing summary

month — 20 seconds

2019-04-18 13:12:09.369895+0200 WordPress[97705:22645475] dispatched network call for month ending on 2019-04-18 11:12:09 +0000
2019-04-18 13:12:32.832780+0200 WordPress[97705:22645475] received summary data
2019-04-18 13:12:32.834390+0200 WordPress[97705:22645475] finished parsing summary

year: — 50 seconds

2019-04-18 13:13:50.190642+0200 WordPress[97705:22645475] dispatched network call for year ending on 2019-04-18 11:13:50 +0000
2019-04-18 13:14:40.553287+0200 WordPress[97705:22645475] received summary data
2019-04-18 13:14:40.554276+0200 WordPress[97705:22645475] finished parsing summary

@jklausa
Copy link
Contributor

jklausa commented Apr 18, 2019

Is there a JPOP team we could ping about maybe changing the request here? for what it's worth, the URL we're currently hitting is

https://public-api.wordpress.com/rest/v1.1/sites/3584907/stats/visits/?locale=en&date=2019-04-18&max=10&period=year&quantity=10&stat_fields=views%2Cvisitors%2Clikes%2Ccomments&unit=year

Copy link
Contributor

@jklausa jklausa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

works fine, I did some investigation why is this so slow and left some comments about it — hopefully they can be of some use!

@ghost
Copy link

ghost commented Apr 18, 2019

@jklausa thanks for this additional investigation!

I took a quick peek to see whether the slowness is my fault, or the actual network taking a long time,

@planarvoid do you all see similar request latency for the longer statistics periods?

using a ages-old tradition of NSLog debugging.

Nothing wrong with that! Although it would be great if we could transition WordPressKit to leverage URLSessionTaskMetrics at some point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Stats Refresh] Period Overview card: populate with real data

2 participants