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

fix: normalizes start date for survey filters #29097

Merged
merged 4 commits into from
Feb 22, 2025
Merged

fix: normalizes start date for survey filters #29097

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4a61fb4
fix: normalizes start date for survey filters
lucasheriques Feb 22, 2025
41ef46c
Merge branch 'master' into fix/normalizes-start-date-for-survey-filters
lucasheriques Feb 22, 2025
4154fec
fix time format
lucasheriques Feb 22, 2025
858973d
cast survey type, remove filters change
lucasheriques Feb 22, 2025
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
41 changes: 25 additions & 16 deletions frontend/src/scenes/surveys/surveyLogic.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,12 @@ function duplicateExistingSurvey(survey: Survey | NewSurvey): Partial<Survey> {

const DATE_FORMAT = 'YYYY-MM-DD HH:mm:ss'

function getSurveyStartDateForQuery(survey: Survey): string {
return survey.start_date
? dayjs(survey.start_date).startOf('day').format(DATE_FORMAT)
: dayjs(survey.created_at).startOf('day').format(DATE_FORMAT)
}

function getSurveyEndDateForQuery(survey: Survey): string {
return survey.end_date
? dayjs(survey.end_date).endOf('day').format(DATE_FORMAT)
Expand Down Expand Up @@ -322,7 +328,7 @@ export const surveyLogic = kea<surveyLogicType>([
surveyUserStats: {
loadSurveyUserStats: async (): Promise<SurveyUserStats> => {
const survey: Survey = values.survey as Survey
const startDate = dayjs(survey.start_date || survey.created_at).format(DATE_FORMAT)
const startDate = getSurveyStartDateForQuery(survey)
const endDate = getSurveyEndDateForQuery(survey)

const query: HogQLQuery = {
Expand Down Expand Up @@ -378,7 +384,7 @@ export const surveyLogic = kea<surveyLogicType>([
}

const survey: Survey = values.survey as Survey
const startDate = dayjs(survey.start_date || survey.created_at).format(DATE_FORMAT)
const startDate = getSurveyStartDateForQuery(survey)
const endDate = getSurveyEndDateForQuery(survey)

const query: HogQLQuery = {
Expand Down Expand Up @@ -430,7 +436,7 @@ export const surveyLogic = kea<surveyLogicType>([
}

const survey: Survey = values.survey as Survey
const startDate = dayjs(survey.start_date || survey.created_at).format(DATE_FORMAT)
const startDate = getSurveyStartDateForQuery(survey)
const endDate = getSurveyEndDateForQuery(survey)

const query: HogQLQuery = {
Expand Down Expand Up @@ -512,7 +518,7 @@ export const surveyLogic = kea<surveyLogicType>([
questionIndex: number
}): Promise<SurveySingleChoiceResults> => {
const survey: Survey = values.survey as Survey
const startDate = dayjs(survey.start_date || survey.created_at).format(DATE_FORMAT)
const startDate = getSurveyStartDateForQuery(survey)
const endDate = getSurveyEndDateForQuery(survey)

const query: HogQLQuery = {
Expand Down Expand Up @@ -556,7 +562,7 @@ export const surveyLogic = kea<surveyLogicType>([
}

const survey: Survey = values.survey as Survey
const startDate = dayjs(survey.start_date || survey.created_at).format(DATE_FORMAT)
const startDate = getSurveyStartDateForQuery(survey)
const endDate = getSurveyEndDateForQuery(survey)

const query: HogQLQuery = {
Expand Down Expand Up @@ -613,7 +619,7 @@ export const surveyLogic = kea<surveyLogicType>([
}

const survey: Survey = values.survey as Survey
const startDate = dayjs(survey.start_date || survey.created_at).format(DATE_FORMAT)
const startDate = getSurveyStartDateForQuery(survey)
const endDate = getSurveyEndDateForQuery(survey)

const query: HogQLQuery = {
Expand Down Expand Up @@ -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),
Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

the code even check for the existence of 'created_at'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

then i think you should remove the if ('created_at' in values.survey) { condition, which wouldn't make sense anymore?

date_to: getSurveyEndDateForQuery(values.survey),
}
actions.setDateRange(dateRange)
Expand Down Expand Up @@ -1142,9 +1148,6 @@ export const surveyLogic = kea<surveyLogicType>([
return null
}
const surveyWithResults = survey as Survey
const startDate = dayjs(surveyWithResults.start_date || surveyWithResults.created_at).format(
'YYYY-MM-DD'
)

return {
kind: NodeKind.DataTableNode,
Expand All @@ -1171,7 +1174,7 @@ export const surveyLogic = kea<surveyLogicType>([
],
orderBy: ['timestamp DESC'],
where: [`event == 'survey sent'`],
after: startDate,
after: getSurveyStartDateForQuery(surveyWithResults),
properties: [
{
type: PropertyFilterType.Event,
Expand Down Expand Up @@ -1419,13 +1422,19 @@ export const surveyLogic = kea<surveyLogicType>([
defaultInterval: [
(s) => [s.survey],
(survey: Survey): IntervalType => {
const start = dayjs(survey.start_date || survey.created_at)
const end = survey.end_date ? dayjs(survey.end_date) : dayjs()
const diffInWeeks = end.diff(start, 'weeks')
const start = getSurveyStartDateForQuery(survey)
const end = getSurveyEndDateForQuery(survey)
const diffInDays = dayjs(end).diff(dayjs(start), 'days')

if (diffInWeeks <= 4) {
if (diffInDays < 2) {
return 'hour'
}
// less than a month
if (diffInDays < 30) {
return 'day'
} else if (diffInWeeks <= 12) {
}
// more than a month, less than 3 months
if (diffInDays < 90) {
return 'week'
}
return 'month'
Expand Down
Loading