Stats: Add stats list to redux tree

[timmyc]

For #5046 this branch seeks to create a new stats state sub-tree to accommodate the data that is currently being managed by the legacy stats-list. This branch builds upon some of the concepts from @jblz 's prior PR on this topic ( #2310 ) but also incorporates approaches taken in the Terms state tree as well.

Stats List State Structure

Since the stats list houses a variety of data from various endpionts, I have opted to segment the state tree of items by siteId and the statsType - which is essentially the method name used on wpcom.js to call out to a particular stats endpoint ( i.e. statsStreak, statsCountryViews etc ).

statsTypes for a givent siteId are then keyed by a stringified-query key that allows for persisting the various payloads we use in stats for various time periods and start/end data combinations. In a similar fashion, the requesting sub-tree keeps track of what API requests are in-flight.

{
    items: {
      `siteId`: {
        `statType`: {
          `queryKey`: {} //API payload data
        }
      }
    },
    requesting: {
      `siteId`: {
        `statType`: {
          `queryKey`: `boolean`
        }
      }
    }
}

Selectors

In the stats-list all API payloads are passed through the stats-parser prior to being stored in localStorage. This allows us to massage the data and format it in such a way to be easily rendered by the stats components.

Some ways to handle this with redux would be to do a similar massage of the data in a component ( which felt odd to me ), or I took the approach here of creating a specific memoized selector like what exists in stats-list. In this branch I implemented getParsedStreakDataForQuery that displays this approach. This feels like a good solution to me due to the ease of testing, and the added value of memoizing the result for speed wins. But please chime in with any opinions here - or alternate approaches.

How To Test

This branch does not modify any existing behavior, it is just setting the stage for further redux greatness in calypso stats. Please refer to the tests in this branch to see the functionality and ensure the tests pass: npm run test-client client/state/stats

TODO

After this branch is in place a query component could be created, and my first area to migrate will be the stats post streak component on the insights page.

[mtias]

Ref: #5046

[timmyc]

Currently in the stats-list we have support for undocumented endpoints here, and instead of copying that, I'd rather migrate those endpoints to wpcom.js as we update all of stats to use redux.

[aduth]

Currently in the stats-list we have support for undocumented endpoints here, and instead of copying that, I'd rather migrate those endpoints to wpcom.js as we update all of stats to use redux.

Would this be a blocker for merging this pull request? Or move forward with it despite being unusable with the assumption that it'll be added to wpcom.js?

[timmyc]

Not a blocker at the moment. One of the undocumented endpoints is not even in use in Calypso. statsInsights is - but there is much work to do between here and there updating stats controller and components to get things working with Redux.

[timmyc]

Automattic/wpcom.js#198 🔑

[timmyc]

/cc @aduth would love your feedback on this one when you have some time!

[aduth]

Was there a reason for creating a separate variable, since the delete call on the line below mutates the original data anyways. Doesn't seem like it'd be an issue to mutate though, but could alternatively consider pulling in Lodash omit to strip the headers property.

[timmyc]

Good call, I will go with omit

[aduth]

I wonder if this would be a good opportunity to split the data we're constructing here into more granular selectors that can be composed together or used in more specific circumstances, e.g. getPostsStatsPerWeekday, getBestStatsWeekday, getPostsStatsWeekdayPercent, getStatsPostsWeekdayMax. Might also help readability of the code in splitting this into well-described functions.

[timmyc]

I like this approach, and I think currently the only bit of data actually used on the component would be getPostsStatsStreak and getStatsPostsStreakMax.

I did notice you don't have ForQuery in any of those - too verbose?

[aduth]

I did notice you don't have ForQuery in any of those - too verbose?

Didn't leave the suffix out intentionally. I don't think I'd mind either way, depending on whether it helps clarify its meaning.

[timmyc]

@aduth in e8be85d I updated the test constants to use snake case and broke out the streak data selectors in to two explicit methods: getSiteStatsMaxPostsByDay, getSiteStatsPostStreakData. I'm keeping getSiteStats... as a prefix on all of these stats list selectors, but opting out of the ForQuery on some of these longer names.

[aduth]

This looks good to me 👍