From e8c468b5fa5ae3c8329cc353870e5ba36cbc6a27 Mon Sep 17 00:00:00 2001 From: Dean Date: Wed, 1 Feb 2023 12:14:10 +0000 Subject: [PATCH 1/4] Added a tenant feature flag for the onboarding tour --- packages/builder/src/helpers/featureFlags.js | 1 + .../src/pages/builder/app/[application]/_layout.svelte | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/builder/src/helpers/featureFlags.js b/packages/builder/src/helpers/featureFlags.js index ae6646bd9f..bc937064f2 100644 --- a/packages/builder/src/helpers/featureFlags.js +++ b/packages/builder/src/helpers/featureFlags.js @@ -4,6 +4,7 @@ import { get } from "svelte/store" export const TENANT_FEATURE_FLAGS = { LICENSING: "LICENSING", USER_GROUPS: "USER_GROUPS", + ONBOADING_TOUR: "ONBOADING_TOUR", } export const isEnabled = featureFlag => { diff --git a/packages/builder/src/pages/builder/app/[application]/_layout.svelte b/packages/builder/src/pages/builder/app/[application]/_layout.svelte index c99776320f..43efe279a8 100644 --- a/packages/builder/src/pages/builder/app/[application]/_layout.svelte +++ b/packages/builder/src/pages/builder/app/[application]/_layout.svelte @@ -2,6 +2,7 @@ import { store, automationStore } from "builderStore" import { roles, flags } from "stores/backend" import { auth } from "stores/portal" + import { TENANT_FEATURE_FLAGS, isEnabled } from "helpers/featureFlags" import { ActionMenu, MenuItem, @@ -68,7 +69,10 @@ } const initTour = async () => { - if (!$auth.user?.onboardedAt) { + if ( + !$auth.user?.onboardedAt && + isEnabled(TENANT_FEATURE_FLAGS.ONBOADING_TOUR) + ) { // Determine the correct step const activeNav = $layout.children.find(c => $isActive(c.path)) const onboardingTour = TOURS[TOUR_KEYS.TOUR_BUILDER_ONBOARDING] From 7242dd41622242fe2a0fa6bf07740ddbc8fcf480 Mon Sep 17 00:00:00 2001 From: Dean Date: Wed, 1 Feb 2023 14:12:43 +0000 Subject: [PATCH 2/4] Updated default feature flags to include onboarding by default. Added tenant overrides to allow exclusion from global features --- charts/budibase/values.yaml | 2 +- hosting/single/runner.sh | 2 +- .../backend-core/src/featureFlags/index.ts | 21 ++++++++++++++++--- packages/builder/src/helpers/featureFlags.js | 2 +- packages/server/Dockerfile | 2 +- packages/server/scripts/dev/manage.js | 2 +- packages/worker/Dockerfile | 2 +- packages/worker/scripts/dev/manage.js | 2 +- 8 files changed, 25 insertions(+), 10 deletions(-) diff --git a/charts/budibase/values.yaml b/charts/budibase/values.yaml index 889d7e9e23..dd75b2daa3 100644 --- a/charts/budibase/values.yaml +++ b/charts/budibase/values.yaml @@ -76,7 +76,7 @@ affinity: {} globals: appVersion: "latest" budibaseEnv: PRODUCTION - tenantFeatureFlags: "*:LICENSING,*:USER_GROUPS" + tenantFeatureFlags: "*:LICENSING,*:USER_GROUPS,*:ONBOARDING_TOUR" enableAnalytics: "1" sentryDSN: "" posthogToken: "phc_bIjZL7oh2GEUd2vqvTBH8WvrX0fWTFQMs6H5KQxiUxU" diff --git a/hosting/single/runner.sh b/hosting/single/runner.sh index 6eebba62b6..3a28cd6e4f 100644 --- a/hosting/single/runner.sh +++ b/hosting/single/runner.sh @@ -10,7 +10,7 @@ declare -a DOCKER_VARS=("APP_PORT" "APPS_URL" "ARCHITECTURE" "BUDIBASE_ENVIRONME [[ -z "${MINIO_URL}" ]] && export MINIO_URL=http://localhost:9000 [[ -z "${NODE_ENV}" ]] && export NODE_ENV=production [[ -z "${POSTHOG_TOKEN}" ]] && export POSTHOG_TOKEN=phc_bIjZL7oh2GEUd2vqvTBH8WvrX0fWTFQMs6H5KQxiUxU -[[ -z "${TENANT_FEATURE_FLAGS}" ]] && export TENANT_FEATURE_FLAGS="*:LICENSING,*:USER_GROUPS" +[[ -z "${TENANT_FEATURE_FLAGS}" ]] && export TENANT_FEATURE_FLAGS="*:LICENSING,*:USER_GROUPS,*:ONBOARDING_TOUR" [[ -z "${ACCOUNT_PORTAL_URL}" ]] && export ACCOUNT_PORTAL_URL=https://account.budibase.app [[ -z "${REDIS_URL}" ]] && export REDIS_URL=localhost:6379 [[ -z "${SELF_HOSTED}" ]] && export SELF_HOSTED=1 diff --git a/packages/backend-core/src/featureFlags/index.ts b/packages/backend-core/src/featureFlags/index.ts index 71e226c976..6e8277ce13 100644 --- a/packages/backend-core/src/featureFlags/index.ts +++ b/packages/backend-core/src/featureFlags/index.ts @@ -36,18 +36,33 @@ export function isEnabled(featureFlag: string) { } export function getTenantFeatureFlags(tenantId: string) { - const flags = [] + let flags: string[] = [] if (TENANT_FEATURE_FLAGS) { const globalFlags = TENANT_FEATURE_FLAGS["*"] - const tenantFlags = TENANT_FEATURE_FLAGS[tenantId] + const tenantFlags = TENANT_FEATURE_FLAGS[tenantId] || [] + + // Explicitly exclude tenants from global features if required. + // Prefix the tenant flag with '!' + const tenantOverrides = tenantFlags.reduce((acc: string[], flag) => { + if (flag.startsWith("!")) { + let stripped = flag.substring(1) + acc.push(stripped) + } + return acc + }, []) if (globalFlags) { flags.push(...globalFlags) } - if (tenantFlags) { + if (tenantFlags.length) { flags.push(...tenantFlags) } + + // Purge any tenant specific overrides + flags = flags.filter(flag => { + return tenantOverrides.indexOf(flag) == -1 && !flag.startsWith("!") + }) } return flags diff --git a/packages/builder/src/helpers/featureFlags.js b/packages/builder/src/helpers/featureFlags.js index bc937064f2..462dae8c54 100644 --- a/packages/builder/src/helpers/featureFlags.js +++ b/packages/builder/src/helpers/featureFlags.js @@ -4,7 +4,7 @@ import { get } from "svelte/store" export const TENANT_FEATURE_FLAGS = { LICENSING: "LICENSING", USER_GROUPS: "USER_GROUPS", - ONBOADING_TOUR: "ONBOADING_TOUR", + ONBOARDING_TOUR: "ONBOARDING_TOUR", } export const isEnabled = featureFlag => { diff --git a/packages/server/Dockerfile b/packages/server/Dockerfile index b55bde7906..d5d516d999 100644 --- a/packages/server/Dockerfile +++ b/packages/server/Dockerfile @@ -12,7 +12,7 @@ ENV COUCH_DB_URL=https://couchdb.budi.live:5984 ENV BUDIBASE_ENVIRONMENT=PRODUCTION ENV SERVICE=app-service ENV POSTHOG_TOKEN=phc_bIjZL7oh2GEUd2vqvTBH8WvrX0fWTFQMs6H5KQxiUxU -ENV TENANT_FEATURE_FLAGS=*:LICENSING,*:USER_GROUPS +ENV TENANT_FEATURE_FLAGS=*:LICENSING,*:USER_GROUPS,*:ONBOARDING_TOUR ENV ACCOUNT_PORTAL_URL=https://account.budibase.app # copy files and install dependencies diff --git a/packages/server/scripts/dev/manage.js b/packages/server/scripts/dev/manage.js index b8566bbf4c..1cc96db2e2 100644 --- a/packages/server/scripts/dev/manage.js +++ b/packages/server/scripts/dev/manage.js @@ -44,7 +44,7 @@ async function init() { BB_ADMIN_USER_EMAIL: "", BB_ADMIN_USER_PASSWORD: "", PLUGINS_DIR: "", - TENANT_FEATURE_FLAGS: "*:LICENSING,*:USER_GROUPS", + TENANT_FEATURE_FLAGS: "*:LICENSING,*:USER_GROUPS,*:ONBOARDING_TOUR", } let envFile = "" Object.keys(envFileJson).forEach(key => { diff --git a/packages/worker/Dockerfile b/packages/worker/Dockerfile index 046b844815..e0cac94eda 100644 --- a/packages/worker/Dockerfile +++ b/packages/worker/Dockerfile @@ -23,7 +23,7 @@ ENV NODE_ENV=production ENV CLUSTER_MODE=${CLUSTER_MODE} ENV SERVICE=worker-service ENV POSTHOG_TOKEN=phc_bIjZL7oh2GEUd2vqvTBH8WvrX0fWTFQMs6H5KQxiUxU -ENV TENANT_FEATURE_FLAGS=*:LICENSING,*:USER_GROUPS +ENV TENANT_FEATURE_FLAGS=*:LICENSING,*:USER_GROUPS,*:ONBOARDING_TOUR ENV ACCOUNT_PORTAL_URL=https://account.budibase.app CMD ["./docker_run.sh"] diff --git a/packages/worker/scripts/dev/manage.js b/packages/worker/scripts/dev/manage.js index a4eaf37162..c993e1fefc 100644 --- a/packages/worker/scripts/dev/manage.js +++ b/packages/worker/scripts/dev/manage.js @@ -28,7 +28,7 @@ async function init() { APPS_URL: "http://localhost:4001", SERVICE: "worker-service", DEPLOYMENT_ENVIRONMENT: "development", - TENANT_FEATURE_FLAGS: "*:LICENSING,*:USER_GROUPS", + TENANT_FEATURE_FLAGS: "*:LICENSING,*:USER_GROUPS,*:ONBOARDING_TOUR", } let envFile = "" Object.keys(envFileJson).forEach(key => { From 04386d01c2ad40dfe2b2f9bcfdc887927e4059c6 Mon Sep 17 00:00:00 2001 From: Dean Date: Thu, 2 Feb 2023 11:38:10 +0000 Subject: [PATCH 3/4] Added new unit tests for feature flags --- .../backend-core/src/featureFlags/index.ts | 30 ++++--- .../featureFlags/tests/featureFlags.spec.ts | 85 +++++++++++++++++++ 2 files changed, 101 insertions(+), 14 deletions(-) create mode 100644 packages/backend-core/src/featureFlags/tests/featureFlags.spec.ts diff --git a/packages/backend-core/src/featureFlags/index.ts b/packages/backend-core/src/featureFlags/index.ts index 6e8277ce13..34ee3599a5 100644 --- a/packages/backend-core/src/featureFlags/index.ts +++ b/packages/backend-core/src/featureFlags/index.ts @@ -6,7 +6,7 @@ import * as tenancy from "../tenancy" * The env var is formatted as: * tenant1:feature1:feature2,tenant2:feature1 */ -function getFeatureFlags() { +export function buildFeatureFlags() { if (!env.TENANT_FEATURE_FLAGS) { return } @@ -27,8 +27,6 @@ function getFeatureFlags() { return tenantFeatureFlags } -const TENANT_FEATURE_FLAGS = getFeatureFlags() - export function isEnabled(featureFlag: string) { const tenantId = tenancy.getTenantId() const flags = getTenantFeatureFlags(tenantId) @@ -37,20 +35,23 @@ export function isEnabled(featureFlag: string) { export function getTenantFeatureFlags(tenantId: string) { let flags: string[] = [] - - if (TENANT_FEATURE_FLAGS) { - const globalFlags = TENANT_FEATURE_FLAGS["*"] - const tenantFlags = TENANT_FEATURE_FLAGS[tenantId] || [] + const envFlags = buildFeatureFlags() + if (envFlags) { + const globalFlags = envFlags["*"] + const tenantFlags = envFlags[tenantId] || [] // Explicitly exclude tenants from global features if required. // Prefix the tenant flag with '!' - const tenantOverrides = tenantFlags.reduce((acc: string[], flag) => { - if (flag.startsWith("!")) { - let stripped = flag.substring(1) - acc.push(stripped) - } - return acc - }, []) + const tenantOverrides = tenantFlags.reduce( + (acc: string[], flag: string) => { + if (flag.startsWith("!")) { + let stripped = flag.substring(1) + acc.push(stripped) + } + return acc + }, + [] + ) if (globalFlags) { flags.push(...globalFlags) @@ -72,4 +73,5 @@ export enum TenantFeatureFlag { LICENSING = "LICENSING", GOOGLE_SHEETS = "GOOGLE_SHEETS", USER_GROUPS = "USER_GROUPS", + ONBOARDING_TOUR = "ONBOARDING_TOUR", } diff --git a/packages/backend-core/src/featureFlags/tests/featureFlags.spec.ts b/packages/backend-core/src/featureFlags/tests/featureFlags.spec.ts new file mode 100644 index 0000000000..1b68959329 --- /dev/null +++ b/packages/backend-core/src/featureFlags/tests/featureFlags.spec.ts @@ -0,0 +1,85 @@ +import { + TenantFeatureFlag, + buildFeatureFlags, + getTenantFeatureFlags, +} from "../" +import env from "../../environment" + +const { ONBOARDING_TOUR, LICENSING, USER_GROUPS } = TenantFeatureFlag + +describe("featureFlags", () => { + beforeEach(() => { + env._set("TENANT_FEATURE_FLAGS", "") + }) + + it("Should return no flags when the TENANT_FEATURE_FLAG is empty", async () => { + let features = buildFeatureFlags() + expect(features).toBeUndefined() + }) + + it("Should generate a map of global and named tenant feature flags from the env value", async () => { + env._set( + "TENANT_FEATURE_FLAGS", + `*:${ONBOARDING_TOUR},tenant1:!${ONBOARDING_TOUR},tenant2:${USER_GROUPS},tenant1:${LICENSING}` + ) + + const parsedFlags: Record = { + "*": [ONBOARDING_TOUR], + tenant1: [`!${ONBOARDING_TOUR}`, LICENSING], + tenant2: [USER_GROUPS], + } + + let features = buildFeatureFlags() + + expect(features).toBeDefined() + expect(features).toEqual(parsedFlags) + }) + + it("Should add feature flag flag only to explicitly configured tenant", async () => { + env._set( + "TENANT_FEATURE_FLAGS", + `*:${LICENSING},*:${USER_GROUPS},tenant1:${ONBOARDING_TOUR}` + ) + + let tenant1Flags = getTenantFeatureFlags("tenant1") + let tenant2Flags = getTenantFeatureFlags("tenant2") + + expect(tenant1Flags).toBeDefined() + expect(tenant1Flags).toEqual([LICENSING, USER_GROUPS, ONBOARDING_TOUR]) + + expect(tenant2Flags).toBeDefined() + expect(tenant2Flags).toEqual([LICENSING, USER_GROUPS]) + }) +}) + +it("Should exclude tenant1 from global feature flag", async () => { + env._set( + "TENANT_FEATURE_FLAGS", + `*:${LICENSING},*:${ONBOARDING_TOUR},tenant1:!${ONBOARDING_TOUR}` + ) + + let tenant1Flags = getTenantFeatureFlags("tenant1") + let tenant2Flags = getTenantFeatureFlags("tenant2") + + expect(tenant1Flags).toBeDefined() + expect(tenant1Flags).toEqual([LICENSING]) + + expect(tenant2Flags).toBeDefined() + expect(tenant2Flags).toEqual([LICENSING, ONBOARDING_TOUR]) +}) + +it("Should explicitly add flags to configured tenants only", async () => { + env._set( + "TENANT_FEATURE_FLAGS", + `tenant1:${ONBOARDING_TOUR},tenant1:${LICENSING},tenant2:${LICENSING}` + ) + + let tenant1Flags = getTenantFeatureFlags("tenant1") + let tenant2Flags = getTenantFeatureFlags("tenant2") + + expect(tenant1Flags).toBeDefined() + expect(tenant1Flags).toEqual([ONBOARDING_TOUR, LICENSING]) + + expect(tenant2Flags).toBeDefined() + expect(tenant2Flags).toEqual([LICENSING]) +}) From e273a0367ac2f8f56860be80581a2853a14acb03 Mon Sep 17 00:00:00 2001 From: Dean Date: Thu, 2 Feb 2023 11:53:49 +0000 Subject: [PATCH 4/4] Fixed typo in feature flag --- .../builder/src/pages/builder/app/[application]/_layout.svelte | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/builder/src/pages/builder/app/[application]/_layout.svelte b/packages/builder/src/pages/builder/app/[application]/_layout.svelte index 43efe279a8..f561bd8ecd 100644 --- a/packages/builder/src/pages/builder/app/[application]/_layout.svelte +++ b/packages/builder/src/pages/builder/app/[application]/_layout.svelte @@ -71,7 +71,7 @@ const initTour = async () => { if ( !$auth.user?.onboardedAt && - isEnabled(TENANT_FEATURE_FLAGS.ONBOADING_TOUR) + isEnabled(TENANT_FEATURE_FLAGS.ONBOARDING_TOUR) ) { // Determine the correct step const activeNav = $layout.children.find(c => $isActive(c.path))