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.

Sign up for GitHub

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

Jump to bottom

test(app-project): run YourStats tests in a non-UTC timezone #6412

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

test(app-project): run YourStats tests in a non-UTC timezone #6412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2104465
test(app-project): refactor YourStats tests to a non-UTC timezone
eatyourgreens Oct 29, 2024
1eb7b17
calculate the first day of the week in UTC
eatyourgreens Oct 29, 2024
d703a43
Document firstDayOfWeek
eatyourgreens Oct 29, 2024
c7c6246
Run the tests in the -06:00 timezone
eatyourgreens Oct 29, 2024
2ba3612
Use UTC times consistently for daily classifications
eatyourgreens Nov 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...ents/YourStats/components/DailyClassificationsChart/DailyClassificationsChartContainer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ function DailyClassificationsChartContainer({
const [year, monthIndex, date] = period.split('-')
const utcDay = Date.UTC(year, monthIndex - 1, date)
const day = new Date(utcDay)
const isToday = day.getUTCDay() === TODAY.getDay()
const isToday = day.getUTCDay() === TODAY.getUTCDay()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Today" is always the current day in Greenwich.

const count = isToday ? counts.today : statsCount
const longLabel = day.toLocaleDateString(locale, { timeZone: 'UTC', weekday: 'long' })
const alt = `${longLabel}: ${count}`
Expand Down
12 changes: 9 additions & 3 deletions packages/app-project/stores/User/UserPersonalization/YourStats/YourStats.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,14 @@ export const statsClient = {
}
}

// https://stackoverflow.com/a/51918448/10951669
function firstDayOfWeek (dateObject, firstDayOfWeekIndex) {
/**
* Find the first day matching a given weekday number, prior to a given UTC date.
* https://stackoverflow.com/a/51918448/10951669
* @param {Date} dateObject search prior to this datetime.
* @param {number} firstDayOfWeekIndex day of the week to find (Sunday is 0.)
* @returns a UTC date object for the first day of the week
*/
function firstDayOfWeek(dateObject, firstDayOfWeekIndex) {
const dayOfWeek = dateObject.getUTCDay()
const firstDayOfWeek = new Date(dateObject)
const diff = dayOfWeek >= firstDayOfWeekIndex
Expand Down Expand Up @@ -74,7 +80,7 @@ const YourStats = types
weekDay.setUTCDate(newDate)
const period = weekDay.toISOString().substring(0, 10)
const { count } = dailyCounts.find(count => count.period.startsWith(period)) || { count: 0, period }
const dayNumber = weekDay.getDay()
const dayNumber = weekDay.getUTCDay()
Copy link
Contributor Author

@eatyourgreens eatyourgreens Oct 29, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This was causing the weekday bug in #2244, but it only triggers when you run getDay() in a time zone other than UTC. Otherwise, getDay() is identical to getUTCDay().

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #2528 for some context on why the code is getting the weekday here.

weeklyStats.push({
count,
dayNumber,
Expand Down
22 changes: 13 additions & 9 deletions packages/app-project/stores/User/UserPersonalization/YourStats/YourStats.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ describe('Stores > YourStats', function () {
sinon.stub(console, 'error')

const MOCK_DAILY_COUNTS = [
{ count: 12, period: '2019-09-29' },
{ count: 12, period: '2019-09-30' },
{ count: 13, period: '2019-10-01' },
{ count: 14, period: '2019-10-02' },
{ count: 10, period: '2019-10-03' },
{ count: 11, period: '2019-10-04' },
{ count: 8, period: '2019-10-05' },
{ count: 15, period: '2019-10-06' }
{ count: 12, period: '2019-09-29T00:00:00.000Z' },
{ count: 12, period: '2019-09-30T00:00:00.000Z' },
{ count: 13, period: '2019-10-01T00:00:00.000Z' },
{ count: 14, period: '2019-10-02T00:00:00.000Z' },
{ count: 10, period: '2019-10-03T00:00:00.000Z' },
{ count: 11, period: '2019-10-04T00:00:00.000Z' },
{ count: 8, period: '2019-10-05T00:00:00.000Z' },
{ count: 15, period: '2019-10-06T00:00:00.000Z' }
Comment on lines +23 to +30
Copy link
Contributor Author

@eatyourgreens eatyourgreens Nov 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

These mocks should match the API responses for real ERAS data, which uses midnight UTC to mark the start of each day.

]

nockScope = nock('https://panoptes-staging.zooniverse.org/api')
Expand Down Expand Up @@ -67,17 +67,21 @@ describe('Stores > YourStats', function () {
let clock

before(function () {
clock = sinon.useFakeTimers({ now: new Date(2019, 9, 1, 12), toFake: ['Date'] })
// Set the local clock to 1am on Tuesday 1 October 2019, UTC.
// Local time for this test will be 7pm, Monday 30 September 2019, CST.
clock = sinon.useFakeTimers(new Date('2019-10-01T01:00:00Z'))
const user = {
id: '123',
login: 'test.user'
}

process.env.TZ = 'America/Chicago'
rootStore.user.set(user)
})

after(function () {
clock.restore()
delete process.env.TZ
})

it('should request user statistics', function () {
Expand Down