From a9b4d0017f7ab418646c57a3039cad835ba1668d Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Wed, 7 Aug 2024 16:59:33 +0100 Subject: [PATCH] add tests for posthog feature flags --- packages/backend-core/src/features/index.ts | 51 ++++++- .../src/features/tests/features.spec.ts | 130 ++++++++++++------ 2 files changed, 135 insertions(+), 46 deletions(-) diff --git a/packages/backend-core/src/features/index.ts b/packages/backend-core/src/features/index.ts index 7a77eb8b56..4a29dc6daf 100644 --- a/packages/backend-core/src/features/index.ts +++ b/packages/backend-core/src/features/index.ts @@ -1,14 +1,15 @@ import env from "../environment" import * as context from "../context" import { cloneDeep } from "lodash" -import { PostHog } from "posthog-node" +import { PostHog, PostHogOptions } from "posthog-node" import { IdentityType } from "@budibase/types" let posthog: PostHog | undefined -export function init() { +export function init(opts?: PostHogOptions) { if (env.POSTHOG_TOKEN) { posthog = new PostHog(env.POSTHOG_TOKEN, { host: "https://us.i.posthog.com", + ...opts, }) } } @@ -18,6 +19,14 @@ abstract class Flag { return new BooleanFlag(defaultValue) } + static string(defaultValue: string): Flag { + return new StringFlag(defaultValue) + } + + static number(defaultValue: number): Flag { + return new NumberFlag(defaultValue) + } + protected constructor(public defaultValue: T) {} abstract parse(value: any): T @@ -37,6 +46,32 @@ class BooleanFlag extends Flag { } } +class StringFlag extends Flag { + parse(value: any) { + if (typeof value === "string") { + return value + } + throw new Error(`could not parse value "${value}" as string`) + } +} + +class NumberFlag extends Flag { + parse(value: any) { + if (typeof value === "number") { + return value + } + + if (typeof value === "string") { + const parsed = parseFloat(value) + if (!isNaN(parsed)) { + return parsed + } + } + + throw new Error(`could not parse value "${value}" as number`) + } +} + // This is the primary source of truth for feature flags. If you want to add a // new flag, add it here and use the `fetch` and `get` functions to access it. // All of the machinery in this file is to make sure that flags have their @@ -46,6 +81,10 @@ const FLAGS = { GOOGLE_SHEETS: Flag.boolean(false), USER_GROUPS: Flag.boolean(false), ONBOARDING_TOUR: Flag.boolean(false), + + _TEST_BOOLEAN: Flag.boolean(false), + _TEST_STRING: Flag.string("default value"), + _TEST_NUMBER: Flag.number(0), } const DEFAULTS = Object.keys(FLAGS).reduce((acc, key) => { @@ -107,6 +146,7 @@ export async function fetch(): Promise { throw new Error(`Feature: ${feature} is not a boolean`) } + // @ts-ignore flags[feature] = value } } @@ -114,7 +154,7 @@ export async function fetch(): Promise { const identity = context.getIdentity() if (posthog && identity?.type === IdentityType.USER) { const posthogFlags = await posthog.getAllFlagsAndPayloads(identity._id) - for (const [name, value] of Object.entries(posthogFlags)) { + for (const [name, value] of Object.entries(posthogFlags.featureFlags)) { const key = name as keyof typeof FLAGS const flag = FLAGS[key] if (!flag) { @@ -124,8 +164,11 @@ export async function fetch(): Promise { continue } + const payload = posthogFlags.featureFlagPayloads?.[name] + try { - flags[key] = flag.parse(value) + // @ts-ignore + flags[key] = flag.parse(payload || value) } catch (err) { // We don't want an invalid PostHog flag to break the app, so we just // log it and continue. diff --git a/packages/backend-core/src/features/tests/features.spec.ts b/packages/backend-core/src/features/tests/features.spec.ts index 9022d568ee..de40e24b3c 100644 --- a/packages/backend-core/src/features/tests/features.spec.ts +++ b/packages/backend-core/src/features/tests/features.spec.ts @@ -2,6 +2,7 @@ import { IdentityContext, IdentityType } from "@budibase/types" import { defaultFlags, fetch, get, Flags, init } from "../" import { context } from "../.." import { setEnv, withEnv } from "../../environment" +import nodeFetch from "node-fetch" import nock from "nock" describe("feature flags", () => { @@ -14,23 +15,23 @@ describe("feature flags", () => { it.each([ { tenant: "tenant1", - flags: "tenant1:ONBOARDING_TOUR", - expected: { ONBOARDING_TOUR: true }, + flags: "tenant1:_TEST_BOOLEAN", + expected: { _TEST_BOOLEAN: true }, }, { tenant: "tenant1", - flags: "tenant1:!ONBOARDING_TOUR", - expected: { ONBOARDING_TOUR: false }, + flags: "tenant1:!_TEST_BOOLEAN", + expected: { _TEST_BOOLEAN: false }, }, { tenant: "tenant1", - flags: "*:ONBOARDING_TOUR", - expected: { ONBOARDING_TOUR: true }, + flags: "*:_TEST_BOOLEAN", + expected: { _TEST_BOOLEAN: true }, }, { tenant: "tenant1", - flags: "tenant2:ONBOARDING_TOUR", - expected: { ONBOARDING_TOUR: false }, + flags: "tenant2:_TEST_BOOLEAN", + expected: { _TEST_BOOLEAN: false }, }, { tenant: "tenant1", @@ -62,7 +63,7 @@ describe("feature flags", () => { it.each([ { tenant: "tenant1", - flags: "tenant1:ONBOARDING_TOUR,tenant1:FOO", + flags: "tenant1:_TEST_BOOLEAN,tenant1:FOO", expected: "Feature: FOO is not an allowed option", }, ])( @@ -75,43 +76,88 @@ describe("feature flags", () => { ) ) - // describe("posthog", () => { - // const identity: IdentityContext = { - // _id: "us_1234", - // tenantId: "budibase", - // type: IdentityType.USER, - // email: "test@example.com", - // firstName: "Test", - // lastName: "User", - // } + describe("posthog", () => { + const identity: IdentityContext = { + _id: "us_1234", + tenantId: "budibase", + type: IdentityType.USER, + email: "test@example.com", + firstName: "Test", + lastName: "User", + } - // let cleanup: () => void + let cleanup: () => void - // beforeAll(() => { - // cleanup = setEnv({ POSTHOG_TOKEN: "test" }) - // init() - // }) + beforeAll(() => { + cleanup = setEnv({ POSTHOG_TOKEN: "test" }) + }) - // afterAll(() => { - // cleanup() - // }) + afterAll(() => { + cleanup() + }) - // beforeEach(() => { - // nock.cleanAll() - // }) + beforeEach(() => { + nock.cleanAll() - // it("should be able to read flags from posthog", () => - // context.doInIdentityContext(identity, async () => { - // nock("https://app.posthog.com") - // .get("/api/feature_flags/tenant/budibase") - // .reply(200, { - // flags: { - // "budibase:onboardingTour": true, - // }, - // }) + // We need to pass in node-fetch here otherwise nock won't get used + // because posthog-node uses axios under the hood. + init({ fetch: nodeFetch }) + }) - // const flags = await fetch() - // expect(flags.ONBOARDING_TOUR).toBe(true) - // })) - // }) + function mockFlags(flags: { + featureFlags?: Record + featureFlagPayloads?: Record + }) { + nock("https://us.i.posthog.com") + .post("/decide/?v=3", body => { + return body.token === "test" && body.distinct_id === "us_1234" + }) + .reply(200, flags) + } + + it("should be able to read flags from posthog", async () => { + mockFlags({ + featureFlags: { + _TEST_BOOLEAN: true, + }, + }) + + await context.doInIdentityContext(identity, async () => { + const flags = await fetch() + expect(flags._TEST_BOOLEAN).toBe(true) + }) + }) + + it("should be able to read flags from posthog with payloads", async () => { + mockFlags({ + featureFlags: { + _TEST_STRING: true, + }, + featureFlagPayloads: { + _TEST_STRING: "test payload", + }, + }) + + await context.doInIdentityContext(identity, async () => { + const flags = await fetch() + expect(flags._TEST_STRING).toBe("test payload") + }) + }) + + it("should be able to read flags from posthog with numbers", async () => { + mockFlags({ + featureFlags: { + _TEST_NUMBER: true, + }, + featureFlagPayloads: { + _TEST_NUMBER: 123, + }, + }) + + await context.doInIdentityContext(identity, async () => { + const flags = await fetch() + expect(flags._TEST_NUMBER).toBe(123) + }) + }) + }) })