code rabbit suggestion and test fixes

This commit is contained in:
Dhruwang
2026-02-10 16:07:53 +05:30
parent 9d7523542a
commit 504a059af5
5 changed files with 232 additions and 111 deletions

View File

@@ -48,7 +48,6 @@ export const EnterpriseLicenseStatus = ({ active, status, environmentId }: Enter
setIsRechecking(true);
try {
const result = await recheckLicenseAction({ environmentId });
console.log({ result });
if (result?.serverError) {
toast.error(result.serverError || t("environments.settings.enterprise.recheck_license_failed"));
return;

View File

@@ -27,6 +27,7 @@ const Page = async (props) => {
}
const licenseState = await getEnterpriseLicense();
const hasLicense = licenseState.status !== "no-license";
const paidFeatures = [
{
@@ -91,7 +92,7 @@ const Page = async (props) => {
activeId="enterprise"
/>
</PageHeader>
{licenseState.active ? (
{hasLicense ? (
<EnterpriseLicenseStatus
active={licenseState.active}
status={licenseState.status}

View File

@@ -11,7 +11,14 @@ import { authenticatedActionClient } from "@/lib/utils/action-client";
import { AuthenticatedActionClientCtx } from "@/lib/utils/action-client/types/context";
import { applyRateLimit } from "@/modules/core/rate-limit/helpers";
import { rateLimitConfigs } from "@/modules/core/rate-limit/rate-limit-configs";
import { clearLicenseCache, fetchLicenseFresh, getCacheKeys, getEnterpriseLicense } from "./lib/license";
import {
FAILED_FETCH_TTL_MS,
FETCH_LICENSE_TTL_MS,
clearLicenseCache,
fetchLicenseFresh,
getCacheKeys,
getEnterpriseLicense,
} from "./lib/license";
const ZRecheckLicenseAction = z.object({
environmentId: ZId,
@@ -65,11 +72,11 @@ export const recheckLicenseAction = authenticatedActionClient
if (freshLicense) {
// Success - cache with full TTL
await cache.set(cacheKeys.FETCH_LICENSE_CACHE_KEY, freshLicense, 24 * 60 * 60 * 1000);
await cache.set(cacheKeys.FETCH_LICENSE_CACHE_KEY, freshLicense, FETCH_LICENSE_TTL_MS);
} else {
// Failure - cache null with short TTL
// The previous result cache is preserved, so grace period will still work
await cache.set(cacheKeys.FETCH_LICENSE_CACHE_KEY, null, 10 * 60 * 1000);
await cache.set(cacheKeys.FETCH_LICENSE_CACHE_KEY, null, FAILED_FETCH_TTL_MS);
}
// Now get the license state - it should use the fresh data we just cached

View File

@@ -98,6 +98,7 @@ describe("License Core Logic", () => {
mockCache.get.mockReset();
mockCache.set.mockReset();
mockCache.del.mockReset();
mockCache.exists.mockReset();
mockCache.withCache.mockReset();
mockLogger.error.mockReset();
mockLogger.warn.mockReset();
@@ -105,9 +106,10 @@ describe("License Core Logic", () => {
mockLogger.debug.mockReset();
// Set up default mock implementations for Result types
// fetchLicense uses get + exists; getPreviousResult uses get with :previous_result key
mockCache.get.mockResolvedValue({ ok: true, data: null });
mockCache.exists.mockResolvedValue({ ok: true, data: false }); // default: cache miss
mockCache.set.mockResolvedValue({ ok: true });
mockCache.withCache.mockImplementation(async (fn) => await fn());
vi.mocked(prisma.response.count).mockResolvedValue(100);
vi.mocked(prisma.organization.findFirst).mockResolvedValue({
@@ -164,16 +166,22 @@ describe("License Core Logic", () => {
const { getEnterpriseLicense } = await import("./license");
const fetch = (await import("node-fetch")).default as Mock;
// Mock cache.withCache to return cached license details (simulating cache hit)
mockCache.withCache.mockResolvedValue(mockFetchedLicenseDetails);
// Mock cache hit: get returns license for status key, exists returns true
mockCache.get.mockImplementation(async (key: string) => {
if (key.includes(":previous_result")) {
return { ok: true, data: null };
}
if (key.includes(":status")) {
return { ok: true, data: mockFetchedLicenseDetails };
}
return { ok: true, data: null };
});
mockCache.exists.mockResolvedValue({ ok: true, data: true });
const license = await getEnterpriseLicense();
expect(license).toEqual(expectedActiveLicenseState);
expect(mockCache.withCache).toHaveBeenCalledWith(
expect.any(Function),
expect.stringContaining("fb:license:"),
expect.any(Number)
);
expect(mockCache.get).toHaveBeenCalledWith(expect.stringContaining("fb:license:"));
expect(mockCache.exists).toHaveBeenCalledWith(expect.stringContaining("fb:license:"));
expect(fetch).not.toHaveBeenCalled();
});
@@ -181,9 +189,7 @@ describe("License Core Logic", () => {
const { getEnterpriseLicense } = await import("./license");
const fetch = (await import("node-fetch")).default as Mock;
// Mock cache.withCache to execute the function (simulating cache miss)
mockCache.withCache.mockImplementation(async (fn) => await fn());
// Default mocks give cache miss (get null, exists false)
fetch.mockResolvedValueOnce({
ok: true,
json: async () => ({ data: mockFetchedLicenseDetails }),
@@ -192,11 +198,8 @@ describe("License Core Logic", () => {
const license = await getEnterpriseLicense();
expect(fetch).toHaveBeenCalledTimes(1);
expect(mockCache.withCache).toHaveBeenCalledWith(
expect.any(Function),
expect.stringContaining("fb:license:"),
expect.any(Number)
);
expect(mockCache.get).toHaveBeenCalledWith(expect.stringContaining("fb:license:"));
expect(mockCache.exists).toHaveBeenCalledWith(expect.stringContaining("fb:license:"));
expect(license).toEqual(expectedActiveLicenseState);
});
@@ -212,11 +215,9 @@ describe("License Core Logic", () => {
version: 1,
};
// Mock cache.withCache to return null (simulating fetch failure)
mockCache.withCache.mockResolvedValue(null);
// Mock cache.get to return previous result when requested
mockCache.get.mockImplementation(async (key) => {
// Cache miss for fetch (get null, exists false) -> fetch fails -> null
// getPreviousResult returns previous result for :previous_result key
mockCache.get.mockImplementation(async (key: string) => {
if (key.includes(":previous_result")) {
return { ok: true, data: mockPreviousResult };
}
@@ -227,7 +228,7 @@ describe("License Core Logic", () => {
const license = await getEnterpriseLicense();
expect(mockCache.withCache).toHaveBeenCalled();
expect(mockCache.get).toHaveBeenCalled();
expect(license).toEqual({
active: true,
features: mockPreviousResult.features,
@@ -250,11 +251,8 @@ describe("License Core Logic", () => {
version: 1,
};
// Mock cache.withCache to return null (simulating fetch failure)
mockCache.withCache.mockResolvedValue(null);
// Mock cache.get to return previous result when requested
mockCache.get.mockImplementation(async (key) => {
// Cache miss -> fetch fails -> null; getPreviousResult returns old previous result
mockCache.get.mockImplementation(async (key: string) => {
if (key.includes(":previous_result")) {
return { ok: true, data: mockPreviousResult };
}
@@ -265,7 +263,7 @@ describe("License Core Logic", () => {
const license = await getEnterpriseLicense();
expect(mockCache.withCache).toHaveBeenCalled();
expect(mockCache.get).toHaveBeenCalled();
expect(mockCache.set).toHaveBeenCalledWith(
expect.stringContaining("fb:license:"),
{
@@ -319,12 +317,7 @@ describe("License Core Logic", () => {
const { getEnterpriseLicense } = await import("./license");
const fetch = (await import("node-fetch")).default as Mock;
// Mock cache.withCache to return null (simulating fetch failure)
mockCache.withCache.mockResolvedValue(null);
// Mock cache.get to return no previous result
mockCache.get.mockResolvedValue({ ok: true, data: null });
// Cache miss -> fetch fails; no previous result (default get returns null)
fetch.mockRejectedValueOnce(new Error("Network error"));
const license = await getEnterpriseLicense();
@@ -397,49 +390,52 @@ describe("License Core Logic", () => {
});
expect(mockCache.get).not.toHaveBeenCalled();
expect(mockCache.set).not.toHaveBeenCalled();
expect(mockCache.withCache).not.toHaveBeenCalled();
expect(mockCache.exists).not.toHaveBeenCalled();
});
test("should handle fetch throwing an error and use grace period or return inactive", async () => {
const { getEnterpriseLicense } = await import("./license");
const fetch = (await import("node-fetch")).default as Mock;
// Mock cache.withCache to return null (simulating fetch failure)
mockCache.withCache.mockResolvedValue(null);
// Mock cache.get to return no previous result
mockCache.get.mockResolvedValue({ ok: true, data: null });
fetch.mockRejectedValueOnce(new Error("Network error"));
const license = await getEnterpriseLicense();
expect(license).toEqual({
active: false,
features: null,
lastChecked: expect.any(Date),
isPendingDowngrade: false,
fallbackLevel: "default" as const,
status: "no-license" as const,
});
});
});
describe("getLicenseFeatures", () => {
test("should return features if license is active", async () => {
// Set up environment before import
vi.stubGlobal("window", undefined);
// Runs after "no-license" test which uses vi.doMock; env may have empty key
vi.resetModules();
vi.doMock("@/lib/env", () => ({
env: {
ENTERPRISE_LICENSE_KEY: "test-license-key",
ENVIRONMENT: "production",
VERCEL_URL: "some.vercel.url",
FORMBRICKS_COM_URL: "https://app.formbricks.com",
HTTPS_PROXY: undefined,
HTTP_PROXY: undefined,
},
}));
// Mock cache.withCache to return license details
mockCache.withCache.mockResolvedValue({
status: "active",
const { getEnterpriseLicense } = await import("./license");
const fetch = (await import("node-fetch")).default as Mock;
// Cache miss -> fetch throws -> no previous result -> handleInitialFailure
fetch.mockRejectedValueOnce(new Error("Network error"));
const license = await getEnterpriseLicense();
expect(license).toEqual({
active: false,
features: expect.objectContaining({
isMultiOrgEnabled: false,
projects: 3,
removeBranding: false,
}),
lastChecked: expect.any(Date),
isPendingDowngrade: false,
fallbackLevel: "default" as const,
status: "unreachable" as const,
});
});
});
describe("getLicenseFeatures", () => {
test("should return features if license is active", async () => {
vi.resetModules();
vi.stubGlobal("window", undefined);
// Mock cache hit for fetchLicense (get returns license, exists true)
const activeLicenseDetails = {
status: "active" as const,
features: {
isMultiOrgEnabled: true,
contacts: true,
@@ -452,9 +448,22 @@ describe("License Core Logic", () => {
spamProtection: true,
ai: true,
auditLogs: true,
multiLanguageSurveys: true,
accessControl: true,
quotas: true,
},
};
mockCache.get.mockImplementation(async (key: string) => {
if (key.includes(":previous_result")) {
return { ok: true, data: null };
}
if (key.includes(":status")) {
return { ok: true, data: activeLicenseDetails };
}
return { ok: true, data: null };
});
// Import after env and mocks are set
mockCache.exists.mockResolvedValue({ ok: true, data: true });
const { getLicenseFeatures } = await import("./license");
const features = await getLicenseFeatures();
expect(features).toEqual({
@@ -469,14 +478,47 @@ describe("License Core Logic", () => {
spamProtection: true,
ai: true,
auditLogs: true,
multiLanguageSurveys: true,
accessControl: true,
quotas: true,
});
});
test("should return null if license is inactive", async () => {
const { getLicenseFeatures } = await import("./license");
// Mock cache.withCache to return expired license
mockCache.withCache.mockResolvedValue({ status: "expired", features: null });
// Mock cache hit with expired license (features can be default for schema)
mockCache.get.mockImplementation(async (key: string) => {
if (key.includes(":previous_result")) {
return { ok: true, data: null };
}
if (key.includes(":status")) {
return {
ok: true,
data: {
status: "expired",
features: {
isMultiOrgEnabled: false,
projects: 3,
twoFactorAuth: false,
sso: false,
whitelabel: false,
removeBranding: false,
contacts: false,
ai: false,
saml: false,
spamProtection: false,
auditLogs: false,
multiLanguageSurveys: false,
accessControl: false,
quotas: false,
},
},
};
}
return { ok: true, data: null };
});
mockCache.exists.mockResolvedValue({ ok: true, data: true });
const features = await getLicenseFeatures();
expect(features).toBeNull();
@@ -485,8 +527,8 @@ describe("License Core Logic", () => {
test("should return null if getEnterpriseLicense throws", async () => {
const { getLicenseFeatures } = await import("./license");
// Mock cache.withCache to throw an error
mockCache.withCache.mockRejectedValue(new Error("Cache error"));
// Mock cache.get to throw so getEnterpriseLicense fails
mockCache.get.mockRejectedValue(new Error("Cache error"));
const features = await getLicenseFeatures();
expect(features).toBeNull();
@@ -499,23 +541,59 @@ describe("License Core Logic", () => {
mockCache.get.mockReset();
mockCache.set.mockReset();
mockCache.del.mockReset();
mockCache.withCache.mockReset();
mockCache.exists.mockReset();
vi.resetModules();
});
test("should use 'browser' as cache key in browser environment", async () => {
vi.stubGlobal("window", {});
// Set up default mock for cache.withCache
mockCache.withCache.mockImplementation(async (fn) => await fn());
// Ensure env has license key (previous "no-license" test may have poisoned env)
vi.doMock("@/lib/env", () => ({
env: {
ENTERPRISE_LICENSE_KEY: "test-license-key",
ENVIRONMENT: "production",
VERCEL_URL: "some.vercel.url",
FORMBRICKS_COM_URL: "https://app.formbricks.com",
HTTPS_PROXY: undefined,
HTTP_PROXY: undefined,
},
}));
// Cache miss so fetch runs; mock get/exists for cache checks
mockCache.get.mockResolvedValue({ ok: true, data: null });
mockCache.exists.mockResolvedValue({ ok: true, data: false });
const fetch = (await import("node-fetch")).default as Mock;
fetch.mockResolvedValueOnce({
ok: true,
json: async () => ({
data: {
status: "active",
features: {
isMultiOrgEnabled: true,
projects: 5,
twoFactorAuth: true,
sso: true,
whitelabel: true,
removeBranding: true,
contacts: true,
ai: true,
saml: true,
spamProtection: true,
auditLogs: true,
multiLanguageSurveys: true,
accessControl: true,
quotas: true,
},
},
}),
} as any);
const { getEnterpriseLicense } = await import("./license");
await getEnterpriseLicense();
expect(mockCache.withCache).toHaveBeenCalledWith(
expect.any(Function),
expect.stringContaining("fb:license:browser:status"),
expect.any(Number)
);
expect(mockCache.get).toHaveBeenCalledWith(expect.stringContaining("fb:license:browser:status"));
expect(mockCache.exists).toHaveBeenCalledWith(expect.stringContaining("fb:license:browser:status"));
});
test("should use 'no-license' as cache key when ENTERPRISE_LICENSE_KEY is not set", async () => {
@@ -534,16 +612,19 @@ describe("License Core Logic", () => {
await getEnterpriseLicense();
// The cache should NOT be accessed if there is no license key
expect(mockCache.get).not.toHaveBeenCalled();
expect(mockCache.withCache).not.toHaveBeenCalled();
expect(mockCache.exists).not.toHaveBeenCalled();
});
test("should use hashed license key as cache key when ENTERPRISE_LICENSE_KEY is set", async () => {
vi.resetModules();
const testLicenseKey = "test-license-key";
vi.stubGlobal("window", undefined);
// Ensure env has license key (restore after "no-license" test)
vi.doMock("@/lib/env", () => ({
env: {
ENTERPRISE_LICENSE_KEY: testLicenseKey,
ENVIRONMENT: "production",
VERCEL_URL: "some.vercel.url",
FORMBRICKS_COM_URL: "https://app.formbricks.com",
HTTPS_PROXY: undefined,
@@ -551,22 +632,49 @@ describe("License Core Logic", () => {
},
}));
// Set up default mock for cache.withCache
mockCache.withCache.mockImplementation(async (fn) => await fn());
mockCache.get.mockResolvedValue({ ok: true, data: null });
mockCache.exists.mockResolvedValue({ ok: true, data: false });
const fetch = (await import("node-fetch")).default as Mock;
fetch.mockResolvedValueOnce({
ok: true,
json: async () => ({
data: {
status: "active",
features: {
isMultiOrgEnabled: true,
projects: 5,
twoFactorAuth: true,
sso: true,
whitelabel: true,
removeBranding: true,
contacts: true,
ai: true,
saml: true,
spamProtection: true,
auditLogs: true,
multiLanguageSurveys: true,
accessControl: true,
quotas: true,
},
},
}),
} as any);
const { hashString } = await import("@/lib/hash-string");
const expectedHash = hashString(testLicenseKey);
const { getEnterpriseLicense } = await import("./license");
await getEnterpriseLicense();
expect(mockCache.withCache).toHaveBeenCalledWith(
expect.any(Function),
expect.stringContaining(`fb:license:${expectedHash}:status`),
expect.any(Number)
);
expect(mockCache.get).toHaveBeenCalledWith(expect.stringContaining(`fb:license:${expectedHash}:status`));
expect(mockCache.exists).toHaveBeenCalledWith(expect.stringContaining(`fb:license:${expectedHash}:status`));
});
});
describe("Error and Warning Logging", () => {
beforeEach(() => {
vi.resetModules();
});
test("should log warning when setPreviousResult cache.set fails (line 176-178)", async () => {
const { getEnterpriseLicense } = await import("./license");
const fetch = (await import("node-fetch")).default as Mock;
@@ -591,10 +699,19 @@ describe("License Core Logic", () => {
},
};
// Mock successful fetch from API
mockCache.withCache.mockResolvedValue(mockFetchedLicenseDetails);
// Cache hit - fetchLicense returns cached license
mockCache.get.mockImplementation(async (key: string) => {
if (key.includes(":previous_result")) {
return { ok: true, data: null };
}
if (key.includes(":status")) {
return { ok: true, data: mockFetchedLicenseDetails };
}
return { ok: true, data: null };
});
mockCache.exists.mockResolvedValue({ ok: true, data: true });
// Mock cache.set to fail when saving previous result
// cache.set fails when setPreviousResult tries to save (called for previous_result key)
mockCache.set.mockResolvedValue({
ok: false,
error: new Error("Redis connection failed"),
@@ -602,7 +719,6 @@ describe("License Core Logic", () => {
await getEnterpriseLicense();
// Verify that the warning was logged
expect(mockLogger.warn).toHaveBeenCalledWith(
{ error: new Error("Redis connection failed") },
"Failed to cache previous result"
@@ -613,10 +729,7 @@ describe("License Core Logic", () => {
const { getEnterpriseLicense } = await import("./license");
const fetch = (await import("node-fetch")).default as Mock;
// Mock cache.withCache to execute the function (simulating cache miss)
mockCache.withCache.mockImplementation(async (fn) => await fn());
// Mock API response with 500 status
// Cache miss -> fetch returns 500
const mockStatus = 500;
fetch.mockResolvedValueOnce({
ok: false,
@@ -626,7 +739,6 @@ describe("License Core Logic", () => {
await getEnterpriseLicense();
// Verify that the API error was logged with correct structure
expect(mockLogger.error).toHaveBeenCalledWith(
expect.objectContaining({
status: mockStatus,
@@ -641,8 +753,7 @@ describe("License Core Logic", () => {
const { getEnterpriseLicense } = await import("./license");
const fetch = (await import("node-fetch")).default as Mock;
// Test with 403 Forbidden
mockCache.withCache.mockImplementation(async (fn) => await fn());
// Cache miss -> fetch returns 403
const mockStatus = 403;
fetch.mockResolvedValueOnce({
ok: false,
@@ -652,7 +763,6 @@ describe("License Core Logic", () => {
await getEnterpriseLicense();
// Verify that the API error was logged with correct structure
expect(mockLogger.error).toHaveBeenCalledWith(
expect.objectContaining({
status: mockStatus,
@@ -675,8 +785,7 @@ describe("License Core Logic", () => {
version: 1,
};
mockCache.withCache.mockResolvedValue(null);
mockCache.get.mockImplementation(async (key) => {
mockCache.get.mockImplementation(async (key: string) => {
if (key.includes(":previous_result")) {
return { ok: true, data: mockPreviousResult };
}
@@ -687,7 +796,6 @@ describe("License Core Logic", () => {
await getEnterpriseLicense();
// Verify that the fallback info was logged
expect(mockLogger.info).toHaveBeenCalledWith(
expect.objectContaining({
fallbackLevel: "grace",
@@ -712,8 +820,9 @@ describe("License Core Logic", () => {
const fetch = (await import("node-fetch")).default as Mock;
// Mock cache.withCache to execute the function (simulating cache miss)
mockCache.withCache.mockImplementation(async (fn) => await fn());
// Cache miss so fetchLicense fetches from server
mockCache.get.mockResolvedValue({ ok: true, data: null });
mockCache.exists.mockResolvedValue({ ok: true, data: false });
fetch.mockResolvedValueOnce({
ok: true,

View File

@@ -37,6 +37,11 @@ const CONFIG = {
},
} as const;
/** TTL in ms for successful license fetch results (24h). Re-export for use in actions. */
export const FETCH_LICENSE_TTL_MS = CONFIG.CACHE.FETCH_LICENSE_TTL_MS;
/** TTL in ms for failed license fetch results (10 min). Re-export for use in actions. */
export const FAILED_FETCH_TTL_MS = CONFIG.CACHE.FAILED_FETCH_TTL_MS;
// Types
type FallbackLevel = "live" | "cached" | "grace" | "default";
@@ -415,9 +420,9 @@ export const fetchLicense = async (): Promise<TEnterpriseLicenseDetails | null>
// Only use cache if:
// 1. Cache lookup succeeded
// 2. Key exists in cache (distinguishes between "not cached" and "cached null")
// 3. Cached value is NOT null (null means previous fetch failed - we should retry, not use cached failure)
if (cached.ok && exists.ok && exists.data && cached.data !== null && cached.data !== undefined) {
// 2. Key exists in cache (distinguishes "not cached" from "cached null")
// 3. Return cached.data (including null) - honors FAILED_FETCH_TTL_MS to suppress retries
if (cached.ok && exists.ok && exists.data) {
return cached.data;
}