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

fix(surveys): Process feature_flag_keys on Survey object #1548

Merged
merged 1 commit into from
Nov 22, 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
113 changes: 113 additions & 0 deletions src/__tests__/surveys.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,44 @@ describe('surveys', () => {
} as unknown as Survey,
]

const surveysWithFeatureFlagKeys: Survey[] = [
{
name: 'survey with feature flags',
id: 'survey-with-flags',
description: 'survey with feature flags description',
type: SurveyType.Popover,
questions: [{ type: SurveyQuestionType.Open, question: 'what do you think?' }],
feature_flag_keys: [
{ key: 'flag1', value: 'linked-flag-key' },
{ key: 'flag2', value: 'survey-targeting-flag-key' },
],
start_date: new Date().toISOString(),
end_date: null,
} as unknown as Survey,
{
name: 'survey with disabled feature flags',
id: 'survey-with-disabled-flags',
description: 'survey with disabled feature flags description',
type: SurveyType.Popover,
questions: [{ type: SurveyQuestionType.Open, question: 'why not?' }],
feature_flag_keys: [
{ key: 'flag1', value: 'linked-flag-key2' },
{ key: 'flag2', value: 'survey-targeting-flag-key2' },
],
start_date: new Date().toISOString(),
end_date: null,
} as unknown as Survey,
{
name: 'survey without feature flags',
id: 'survey-without-flags',
description: 'survey without feature flags description',
type: SurveyType.Popover,
questions: [{ type: SurveyQuestionType.Open, question: 'any thoughts?' }],
start_date: new Date().toISOString(),
end_date: null,
} as unknown as Survey,
]

beforeEach(() => {
surveysResponse = { surveys: firstSurveys }

Expand Down Expand Up @@ -652,6 +690,22 @@ describe('surveys', () => {
expect(data).toEqual([activeSurvey, surveyWithSelector, surveyWithEverything])
})
})

it('returns only surveys with enabled feature flags', () => {
surveysResponse = { surveys: surveysWithFeatureFlagKeys }

surveys.getActiveMatchingSurveys((data) => {
// Should include:
// - survey with enabled flags (survey-with-flags)
// - survey without flags (survey-without-flags)
// Should NOT include:
// - survey with disabled flags (survey-with-disabled-flags)
expect(data.length).toBe(2)
expect(data.map((s) => s.id)).toContain('survey-with-flags')
expect(data.map((s) => s.id)).toContain('survey-without-flags')
expect(data.map((s) => s.id)).not.toContain('survey-with-disabled-flags')
})
})
})

describe('shuffling questions', () => {
Expand Down Expand Up @@ -1172,4 +1226,63 @@ describe('surveys', () => {
)
})
})

describe('checkFlags', () => {
it('should return true when no feature flags are specified', () => {
const survey = { id: '123', questions: [] } as Survey
const result = surveys.checkFlags(survey)
expect(result).toBe(true)
})

it('should return true when all feature flags are enabled', () => {
const survey = {
id: '123',
questions: [],
feature_flag_keys: [
{ key: 'flag1', value: 'flag-1' },
{ key: 'flag2', value: 'flag-2' },
],
} as Survey

jest.spyOn(instance.featureFlags, 'isFeatureEnabled').mockImplementation(() => true)

const result = surveys.checkFlags(survey)
expect(result).toBe(true)
})

it('should return false when any feature flag is disabled', () => {
const survey = {
id: '123',
questions: [],
feature_flag_keys: [
{ key: 'flag1', value: 'flag-1' },
{ key: 'flag2', value: 'flag-2' },
],
} as Survey

jest.spyOn(instance.featureFlags, 'isFeatureEnabled').mockImplementation((flag) =>
flag === 'flag-1' ? true : false
)

const result = surveys.checkFlags(survey)
expect(result).toBe(false)
})

it('should ignore feature flags with missing key or value', () => {
const survey = {
id: '123',
questions: [],
feature_flag_keys: [
{ key: '', value: 'flag-1' },
{ key: 'flag2', value: '' },
{ key: 'flag3', value: 'flag-3' },
],
} as Survey

jest.spyOn(instance.featureFlags, 'isFeatureEnabled').mockImplementation(() => true)

const result = surveys.checkFlags(survey)
expect(result).toBe(true)
})
})
})
6 changes: 6 additions & 0 deletions src/posthog-surveys-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ export interface Survey {
name: string
description: string
type: SurveyType
feature_flag_keys:
| {
key: string
value?: string
}[]
| null
linked_flag_key: string | null
targeting_flag_key: string | null
internal_targeting_flag_key: string | null
Expand Down
27 changes: 24 additions & 3 deletions src/posthog-surveys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,12 @@ export class PostHogSurveys {
// get all the surveys that have been activated so far with user actions.
const activatedSurveys: string[] | undefined = this._surveyEventReceiver?.getSurveys()
const targetingMatchedSurveys = conditionMatchedSurveys.filter((survey) => {
if (!survey.linked_flag_key && !survey.targeting_flag_key && !survey.internal_targeting_flag_key) {
if (
!survey.linked_flag_key &&
!survey.targeting_flag_key &&
!survey.internal_targeting_flag_key &&
!survey.feature_flag_keys?.length
) {
return true
}
const linkedFlagCheck = survey.linked_flag_key
Expand All @@ -199,16 +204,32 @@ export class PostHogSurveys {
survey.internal_targeting_flag_key && !overrideInternalTargetingFlagCheck
? this.instance.featureFlags.isFeatureEnabled(survey.internal_targeting_flag_key)
: true

const flagsCheck = this.checkFlags(survey)
return (
linkedFlagCheck && targetingFlagCheck && internalTargetingFlagCheck && eventBasedTargetingFlagCheck
linkedFlagCheck &&
targetingFlagCheck &&
internalTargetingFlagCheck &&
eventBasedTargetingFlagCheck &&
flagsCheck
)
})

return callback(targetingMatchedSurveys)
}, forceReload)
}

checkFlags(survey: Survey): boolean {
if (!survey.feature_flag_keys?.length) {
return true
}

return survey.feature_flag_keys.every(({ key, value }) => {
if (!key || !value) {
return true
}
return this.instance.featureFlags.isFeatureEnabled(value)
})
}
getNextSurveyStep(survey: Survey, currentQuestionIndex: number, response: string | string[] | number | null) {
const question = survey.questions[currentQuestionIndex]
const nextQuestionIndex = currentQuestionIndex + 1
Expand Down
Loading