search

hello / suripu

Suripu スリープ tracks data collected by sensors during sleep.
26 stars 7 forks source link

test of insight resource #1887

Closed jykfan closed 7 years ago

jykfan commented 7 years ago

@jnorgan

cc @pims

jnorgan commented 7 years ago

Gave feedback in person, but will document here. It looks as though writing tests to verify that getInsightsByDate() cannot return an insight in the future and trimming off the '.plusDays(1)' from the queryDate should give us reasonable certainty that we will not pull an insight for a future date as is the case with Marketing insights which are generated for a future UTC timestamp (corrected for user timezone).

jnorgan commented 7 years ago

Also, the tests should verify that the truncation of the timestamp for the DATE_CATEGORY_ATTRIBUTE_NAME doesn't result in a user seeing an insight before we intend for them to see it. For example, if a user is in PST(-8) and we expect to display an insight to this user at 8:30pm (02/14) their time (2017-02-15 04:30:00Z), what would happen? Would they be able to see this message at 2017-02-15 00:00:00Z (4pm PST)?

jykfan commented 7 years ago

@jnorgan agreed. On the second point, actually, we CANNOT guarantee timestamp-granularity in publication time.

We can only guarantee date-granularity at this time :/

jnorgan commented 7 years ago

Since there isn't much else to test in this endpoint, I think these tests of getInsightsByDate() meet the need to ensure future insights don't get returned. I still think there will be issues with users seeing an insight before it is time since we rely on Now (UTC) and a truncated 'date_category' range_key.

pims commented 7 years ago

Ok. 👍🏼