fix: normalizes start date for survey filters #29097
Conversation
There was a problem hiding this comment.
PR Summary
This PR addresses timezone-related inconsistencies in survey filters by normalizing start date handling across different timezones, similar to existing end date normalization.
- Introduced
getSurveyStartDateForQueryfunction in/frontend/src/scenes/surveys/surveyLogic.tsxto standardize date handling usingstartOf('day') - Updated survey query references to use the new normalization function for consistent date filtering
- Added timezone-specific test cases in
/frontend/src/scenes/surveys/surveyLogic.test.tsto verify consistent behavior across different timezones
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings | Greptile
|
Size Change: 0 B Total Size: 9.71 MB ℹ️ View Unchanged
|
| @@ -717,7 +723,7 @@ export const surveyLogic = kea<surveyLogicType>([ | |||
| // Initialize date range based on survey dates when survey is loaded | |||
| if ('created_at' in values.survey) { | |||
| const dateRange = { | |||
| date_from: dayjs(values.survey.created_at).startOf('day').format(DATE_FORMAT), | |||
| date_from: getSurveyStartDateForQuery(values.survey), | |||
There was a problem hiding this comment.
so here it was using created_at only, but the new function uses start and created, with a preference for start, I believe this would be a bug?
There was a problem hiding this comment.
the code even check for the existence of 'created_at'
There was a problem hiding this comment.
all the other dates have always used the start_date as the default date, this was the only one that was using only created_at. so it's more about doing the same behavior to this date range as well
There was a problem hiding this comment.
then i think you should remove the if ('created_at' in values.survey) { condition, which wouldn't make sense anymore?
Problem
similar to how we did it for the end date filter, this will normalize the start date filter to prevent issues where different timezones compute different data
will fix this bug reported internally
Does this work well for both Cloud and self-hosted?
yes
How did you test this code?
smoke tests locally; tested with changing my macbook to different timezones and date remains the same