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

lib-user: Use a getDefaultDateRange() utility #6420

Merged
merged 2 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 7 additions & 4 deletions packages/lib-user/dev/components/App/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,24 @@ import {
UserHome,
UserStats
} from '@components'
import { getDefaultDateRange, getStatsDateString } from '../../../src/utils'

const isBrowser = typeof window !== 'undefined'
const todayUTC = new Date().toISOString().substring(0, 10)
const sevenDaysAgoUTC = new Date(new Date().setDate(new Date().getDate() - 6)).toISOString().substring(0, 10)
const today = new Date()
const todayUTC = getStatsDateString(today)

if (isBrowser) {
auth.checkCurrent()
}

const DEFAULT_DATE_RANGE = getDefaultDateRange()

function App({
endDate = todayUTC,
endDate = DEFAULT_DATE_RANGE.endDate,
groups = null,
joinToken = null,
projectId = undefined,
startDate = sevenDaysAgoUTC,
startDate = DEFAULT_DATE_RANGE.startDate,
updateQueryParams = () => true,
users = null
}) {
Expand Down
6 changes: 2 additions & 4 deletions packages/lib-user/src/components/Certificate/Certificate.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
} from '@components/shared'

import { formatDateRange } from './helpers/formatDateRange'
import { getDefaultDateRange } from '@utils'

const PrintableBox = styled(Box)`
font-size: 16px;
Expand Down Expand Up @@ -57,10 +58,7 @@ const PrintableBox = styled(Box)`
}
`

const DEFAULT_DATE_RANGE = {
endDate: undefined,
startDate: undefined
}
const DEFAULT_DATE_RANGE = getDefaultDateRange()

function handleClickPrint() {
window.print()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,13 @@ import {

import {
convertStatsSecondsToHours,
getDateInterval
getDateInterval,
getDefaultDateRange
} from '@utils'

import Certificate from './Certificate'

const DEFAULT_DATE_RANGE = {
endDate: null,
startDate: null
}
const DEFAULT_DATE_RANGE = getDefaultDateRange()
const STATS_ENDPOINT = '/classifications/users'

function CertificateContainer({
Expand Down Expand Up @@ -52,7 +50,7 @@ function CertificateContainer({
} else {
statsQuery.project_id = parseInt(selectedProject)
}

const {
data: stats,
error: statsError,
Expand All @@ -71,7 +69,7 @@ function CertificateContainer({
cards: true,
id: selectedProject
})

const error = userError || statsError || projectsError
const hours = convertStatsSecondsToHours(stats?.time_spent) || 0
const loading = userLoading || statsLoading || projectsLoading
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@
import { bool, func, shape, string } from 'prop-types'

import { GroupContainer } from '@components/shared'
import { getDefaultDateRange } from '../../utils'

import GroupStats from './GroupStats'

const DEFAULT_DATE_RANGE = {
endDate: undefined,
startDate: undefined
}
const DEFAULT_DATE_RANGE = getDefaultDateRange()
const DEFAULT_HANDLER = () => true

function GroupStatsContainer({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { shape, string } from 'prop-types'
import { useStats } from '@hooks'
import {
getDateInterval,
getDefaultDateRange,
getStatsDateString
} from '@utils'
import StatsTabs from './StatsTabs'
Expand All @@ -20,25 +21,19 @@ const formatStatsPreview = (thisWeekData, allTimeData) => {
}
}

const DEFAULT_DATE_RANGE = getDefaultDateRange() // last 7 days

export default function StatsTabsContainer({ user }) {
const today = new Date()
const todayUTC = getStatsDateString(today)

const thisWeekStart = new Date()
thisWeekStart.setUTCDate(today.getUTCDate() - 6)
const thisWeekQuery = getDateInterval({
endDate: todayUTC,
startDate: getStatsDateString(thisWeekStart)
})
const thisWeekQuery = getDateInterval(DEFAULT_DATE_RANGE)
thisWeekQuery.project_contributions = true
const { data: thisWeekData } = useStats({
sourceId: user?.id,
query: thisWeekQuery
})

const allTimeQuery = getDateInterval({
endDate: todayUTC,
startDate: getStatsDateString(user?.created_at)
endDate: getStatsDateString(new Date()), // Today's UTC date string like 24-09-23.
Copy link
Contributor

Choose a reason for hiding this comment

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

Pedantic, but not sure if the 20 in 2024 should be included

Suggested change
endDate: getStatsDateString(new Date()), // Today's UTC date string like 24-09-23.
endDate: getStatsDateString(new Date()), // Today's UTC date string like 2024-09-23.

startDate: getStatsDateString(user?.created_at) // Limit stats range to when the user acccount was created in panoptes.
})
allTimeQuery.project_contributions = true
const { data: allTimeData } = useStats({
Expand Down
20 changes: 9 additions & 11 deletions packages/lib-user/src/components/UserStats/UserStatsContainer.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,13 @@ import {
} from '@hooks'

import {
getDateInterval
getDateInterval,
getDefaultDateRange
} from '@utils'

import UserStats from './UserStats'

const DEFAULT_DATE_RANGE = {
endDate: undefined,
startDate: undefined
}
const DEFAULT_DATE_RANGE = getDefaultDateRange()
const DEFAULT_HANDLER = () => true
const STATS_ENDPOINT = '/classifications/users'

Expand All @@ -40,12 +38,12 @@ function UserStatsContainer({
login,
requiredUserProperty: 'created_at'
})

// fetch all projects stats, used by projects select and top projects regardless of selected project
const allProjectsStatsQuery = getDateInterval(selectedDateRange)
allProjectsStatsQuery.project_contributions = true
allProjectsStatsQuery.time_spent = true

const {
data: allProjectsStats,
error: statsError,
Expand All @@ -55,12 +53,12 @@ function UserStatsContainer({
sourceId: paramsValidationMessage ? null : user?.id,
query: allProjectsStatsQuery
})

// fetch individual project stats
const projectStatsQuery = getDateInterval(selectedDateRange)
projectStatsQuery.project_id = parseInt(selectedProject)
projectStatsQuery.time_spent = true

const {
data: projectStats,
error: projectStatsError,
Expand All @@ -70,10 +68,10 @@ function UserStatsContainer({
sourceId: selectedProject ? user?.id : null,
query: projectStatsQuery
})

// fetch projects
const projectIds = allProjectsStats?.project_contributions?.map(project => project.project_id)

const {
data: projects,
error: projectsError,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@ import { Loader, MovableModal, SpacedText } from '@zooniverse/react-components'
import { Anchor, Box, Calendar, ResponsiveContext, Text } from 'grommet'
import { arrayOf, bool, func, number, shape, string } from 'prop-types'
import { useCallback, useContext, useEffect, useState } from 'react'
import Link from 'next/link'

import {
convertStatsSecondsToHours,
getDefaultDateRange,
getStatsDateString
} from '@utils'

Expand All @@ -22,13 +24,10 @@ import {
StyledTab
} from './components'
import { getDateRangeSelectOptions, getProjectSelectOptions } from './helpers'
import Link from 'next/link'


const DEFAULT_HANDLER = () => true
const DEFAULT_DATE_RANGE = {
endDate: null,
startDate: null
}
const DEFAULT_DATE_RANGE = getDefaultDateRange()
const DEFAULT_STATS = {
data: [],
time_spent: 0,
Expand Down Expand Up @@ -67,7 +66,7 @@ function MainContent({
const size = useContext(ResponsiveContext)

const hoursSpent = convertStatsSecondsToHours(stats?.time_spent)

const noStats = !stats?.data?.length

const sourceCreatedAtDate = getStatsDateString(source?.created_at)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
getStatsDateString
getStatsDateString,
getDefaultDateRange
} from '@utils'

import { formatSelectOptionDateLabel } from './formatSelectOptionDateLabel'
Expand Down Expand Up @@ -41,10 +42,7 @@ function getPresetSelectOptions({ sourceCreatedAtDate, today }) {
]
}

const DEFAULT_DATE_RANGE = {
endDate: getStatsDateString(new Date()),
startDate: getStatsDateString(new Date(new Date().setUTCDate(new Date().getUTCDate() - 6)))
}
const DEFAULT_DATE_RANGE = getDefaultDateRange()

export function getDateRangeSelectOptions({
sourceCreatedAtDate = '',
Expand All @@ -53,14 +51,14 @@ export function getDateRangeSelectOptions({
}) {
const today = new Date()
const todayUTC = getStatsDateString(today)

const dateRangeOptions = getPresetSelectOptions({ sourceCreatedAtDate, today })

let selectedDateRangeOption = dateRangeOptions.find(option =>
(selectedDateRange.endDate === todayUTC) &&
(option.value === selectedDateRange.startDate)
)

if (!selectedDateRangeOption && !paramsValidationMessage) {
const customDateRangeOption = {
label: `${formatSelectOptionDateLabel(selectedDateRange)}`,
Expand All @@ -74,6 +72,6 @@ export function getDateRangeSelectOptions({
value: 'custom'
})
}

return { dateRangeOptions, selectedDateRangeOption }
}
49 changes: 18 additions & 31 deletions packages/lib-user/src/utils/getDateInterval.js
Original file line number Diff line number Diff line change
@@ -1,42 +1,29 @@
import { getStatsDateString } from '@utils'

export function getDateInterval({ endDate, startDate }) {
const today = new Date()
const todayUTC = getStatsDateString(today)
const end_date = endDate ? endDate : todayUTC
const defaultStartDate = new Date()
defaultStartDate.setUTCDate(today.getUTCDate() - 6)
const start_date = startDate ? startDate : getStatsDateString(defaultStartDate)

const differenceInDays = (new Date(end_date) - new Date(start_date)) / (1000 * 60 * 60 * 24)

/*
ERAS accepts queries in time interval "buckets" of day, week, month, or year.
*/
function getInterval(differenceInDays) {
if (differenceInDays <= 31) {
return {
end_date,
period: 'day',
start_date
}
return 'day'
}

if (differenceInDays <= 183) {
return {
end_date,
period: 'week',
start_date
}
return 'week'
}

if (differenceInDays <= 1460) {
return {
end_date,
period: 'month',
start_date
}
return 'month'
}

return 'year'
}

export function getDateInterval({ endDate, startDate }) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if we need to 1) check endDate or startDate are the date strings we expect and/or 2) set default values (maybe based on getDefaultDateRange)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

getDateInterval() is only ever called with an object containing endDate and startDate. Checking for those strings and setting default values inside getDateInterval() is how the two actions became intertwined 1) Getting the interval of day, week, month, or year, and 2) setting a default endDate and startDate. The intertwining of those two actions is giving lib-user a bit of spaghetti code.

getDateInterval() is more like formatMyErasQueryForMe(). Instead of using it has a catch-all for cases where endDate and startDate are undefined, I'd rather it be a one-purpose function. You've already type checked endDate and startDate in app-root's App Router pages, and I assume grommet's Calendar component has its own type checking for date range variables.

Does that sound like a good plan to you? I'm definitely willing to disregard this PR in favor of keeping the old getDateInterval() if you prefer, but the above mentioned intertwining of actions and the repeated setting of default props with endDate and startDate as "undefined" feels like a bit of a code smell to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

That sounds good to me! I think the separate functions are preferable.

// Get new Date timestamps based on UTC strings, and then convert from ms to days.
const differenceInDays = (new Date(endDate) - new Date(startDate)) / (1000 * 60 * 60 * 24)

return {
end_date,
period: 'year',
start_date
}
end_date: endDate,
period: getInterval(differenceInDays),
start_date: startDate
}
}
36 changes: 36 additions & 0 deletions packages/lib-user/src/utils/getDefaultDateRange.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { getStatsDateString } from '@utils'

/*
Get the default end date (UTC) for stats query
*/
function getDefaultEndDate() {
// Construct a new Javascript Date object
const today = new Date()

// .toISOString() in this function returns a UTC string like 2024-05-05
return getStatsDateString(today)
}

/*
Get the default start date (UTC) for stats query
*/
function getDefaultStartDate() {
// Construct a new Javascript Date object
const defaultStartDate = new Date()

// .getUTCDate() returns numeric day of the month (UTC).
// If a negative number is provided to .setUTCDate(), the date will be
// set counting backwards from the last day of the previous month.
const sevenDaysAgo = defaultStartDate.getUTCDate() - 6
defaultStartDate.setUTCDate(sevenDaysAgo)

// .toISOString() in this function returns a UTC string
return getStatsDateString(defaultStartDate)
}

export function getDefaultDateRange() {
return {
endDate: getDefaultEndDate(),
startDate: getDefaultStartDate()
}
}
6 changes: 5 additions & 1 deletion packages/lib-user/src/utils/getStatsDateString.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
/*
Stats dates are always handled in UTC strings such as 24-09-23.
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to other comment

Suggested change
Stats dates are always handled in UTC strings such as 24-09-23.
Stats dates are always handled in UTC strings such as 2024-09-23.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shoot I just realized I never included these suggested changes. I'll include it in #6472

*/

export function getStatsDateString(date) {
if (date instanceof Date) {
return date.toISOString().substring(0, 10)
return date.toISOString().substring(0, 10) // .toISOString returns UTC
} else {
return date?.substring(0, 10)
}
Expand Down
Loading