The mechanism in Stats for filtering the currently selected data point in a set uses a date string formatted according to the unit.
For example, for unit = 'month'
the dates returned in each object of the data array should be YYYY-MM
.
Currently^^^, the response body.date
is good, but body.data[ 0 ].date
should similarly be formatted.
fwiw i think this is a common pattern in all the other stats endpoints, where the date is still returned for unit = month as the first day of the period. Here is a snippet of the visits endpoint response for unit=month
Yeah I was thinking about that as well, when I was working with the other stats endpoints today. They return a period
with the date in each response. Should I update that here as well @psealock?
Our new endpoints follow this pattern. Here is orders
by month
When filtering over this data, keeping it in terms of a unitDate
prevents errors I think
They return a period with the date in each response. Should I update that here as well
I don't think it'll be needed.
Cool - I didn't know that about the new woo stats endpoints, that definitely makes parsing easier.
Looking at the orders
data^^^ I see the date has been placed inside the array. We have
{
fields: [ 'sales' ],
data: [
{ date: '2018', data: [ 99 ] },
]
}
Maybe we should follow the same pattern:
{
fields: [ 'period', 'sales' ],
data: [
['2018', 99 ]
]
}
@justinshreve , is this what you were asking?
@psealock Yeah that's what I meant!
Posted a patch at D10401-code.
@psealock Looking at it now (the placing inside the array issue) does it actually make sense to follow the
data: [
['2018', ... ]
]
pattern?
Since we return a row per referrer, we can end up with multiple entries per date. This endpoint is different from the orders endpoint in that regard.
i.e
data: [
['2018-01', 'facebook.com', 99 ],
['2018-01', 'instagram.com', 33 ],
// etc...
]
You can see what I mean in the patch I linked above.
Yup, I see what you mean. Sorry Justin for not thinking this through before having you get started.
On the client, it means the difference between filtering through all the data vs finding one of (at most) 30 elements in the data array
// currently
const selectedData = find( data, d => d.date === selectedDate ); // data.length <= 30
// Patch D10401
const selectedData = data.filter( d => d.date === selectedDate ); // data.length === ∞
Potentially, it may result in expensive re-renders if we're looking at an entire month's worth of data.
@psealock so can we continue with the endpoint as-is or do we need to make some changes still? Justin is out for a few days now so I just want to see what needs to be done to keep things moving forward. I'm happy to help out with API changes too.
@timmyc We still need the part of the D10401 that returns the different date formats.
i.e. 2017-04
instead of 2017-04-01
when month is passed, 2017
when year is passed, etc.
I should have time to tackle this Monday if you don't, and I believe Paul said he can work with the day view for now.
@timmyc Known bugs are being tracked here, https://github.com/Automattic/wp-calypso/projects/26 in the MVP Bugs column.
No issues are blockers for me. We have lots of corner cases to examine as you pointed out in https://github.com/Automattic/wp-calypso/pull/21752#issuecomment-368068334
Updated and addressed in the new D10460-code.