Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add stats to the global redux store #2310

Closed
wants to merge 6 commits into from
Closed

Add stats to the global redux store #2310

wants to merge 6 commits into from

Conversation

jblz
Copy link
Member

@jblz jblz commented Jan 12, 2016

User Stats State

A module for managing the data which backs a user or site's Stats reporting features.

Action Creators

  • createFetchActionsForModule

Actions

  • fetchSiteStats

Reducers

  • items
  • fetchingItems

Selectors

  • getStatsItem
  • isStatsItemFetching

Utils

  • getStatsTypesByModule
  • getCompositeKey
  • normalizeParams
  • isDefined

Still @TODO:

  • Persistence / injecting initial state
  • Full feature parity with StatsList:
    • API timing
    • Error handling
    • Request retrying
  • Tests
  • Finish moving components away from StatsList

This is a work-in-progress and will be filled in as the implementation develops.

@jblz jblz added [Type] Enhancement [Feature] Stats Everything related to our analytics product at /stats/ [Pri] Normal Schedule for the next available opportuinity. [Status] In Progress labels Jan 12, 2016
@jblz jblz self-assigned this Jan 12, 2016
@jblz jblz added this to the Stats: Maintenance milestone Jan 19, 2016
@jblz jblz force-pushed the add/stats-redux branch 5 times, most recently from 06f900e to 3a22aa2 Compare January 27, 2016 03:06

case 'searchterms':
summaryList = new StatsList( { siteID: siteId, statType: 'statsSearchTerms', period: activeFilter.period, date: endDate, max: 0, domain: site.slug } );
fetchActions = createFetchActionsForModule( module, activeFilter, endDate, { post: queryOptions.post } );
Copy link
Member Author

Choose a reason for hiding this comment

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

Check to see if this post param applies to both actions... I'm guessing not.

TODO: don't accept the extra params to createFetchActionsForModule & just explicitly modify the fetch actions here for exceptions.

@jblz jblz force-pushed the add/stats-redux branch 3 times, most recently from e2f98e8 to e02f260 Compare January 27, 2016 03:43
const params = {
siteID,
statType,
options: {
Copy link
Member Author

Choose a reason for hiding this comment

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

If we pass postID here as well, we can automate this in getStatsItem & isStatsItemFetching.

Ideally, we wrap connect in our own util function statsConnect (or something) that handles this boilerplate for us in each component.

jblz added 6 commits January 27, 2016 14:35
Initial commit includes a basic action & reducer to get started working with the store.
Now with selectable per-query states for response values & a composite-key-based `isLoading` value
… controller

* Hooked up the individual post view (`StatsPostDetail`) to the redux flow by mapping redux state to component props
* Removed usage of `StatsList` in `StatsPostDetail` & all child components
* Use domain from the URL if the site id is not available
* Block actions without a site id or domain (fixes #2400)
* Break selectors and utils into their own files
* Use deterministic composite keys instead of nested objects as store values
* Hook up most of the countryViews module tree
* Generalize modules' connections between statTypes & fetchActions to their `moduleState` prop
* Adapt `StatsDownloadCsv` & add util function: `csvData`

export function getStatsItem( state, params ) {
const _params = normalizeParams( params );
const key = getCompositeKey( _params );
Copy link
Member Author

Choose a reason for hiding this comment

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

Note to self: try out the deterministic-stringify lib here.

@aduth
Copy link
Contributor

aduth commented Mar 4, 2016

This pull request will need to be updated with the merger of #3773:

  • No need to specify custom Makefile in client/state
    • Instead, require your tests directly in client/state/test/index.js
  • No need to specify Chai middleware (already configured in root test suite)
  • Do not call nock.restore() in your test tear down. Instead, call nock.cleanAll() (the former will disable nock altogether for the remainder of tests)

@lancewillett
Copy link
Contributor

Needs a refresh still.

@jblz
Copy link
Member Author

jblz commented Jul 31, 2016

Closing since #5743 was merged

@jblz jblz closed this Jul 31, 2016
@jblz jblz deleted the add/stats-redux branch July 31, 2016 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Stats Everything related to our analytics product at /stats/ [Pri] Normal Schedule for the next available opportuinity. [Type] Enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants