Skip to content

Commit

Permalink
feat: Implement rebac feature flag via query params
Browse files Browse the repository at this point in the history
  • Loading branch information
Ninfa-Jeon committed Mar 3, 2025
1 parent 10e6d25 commit 32d9e8f
Show file tree
Hide file tree
Showing 6 changed files with 215 additions and 2 deletions.
2 changes: 2 additions & 0 deletions src/components/LogIn/LogIn.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import AuthenticationButton from "components/AuthenticationButton";
import Logo from "components/Logo";
import ToastCard from "components/ToastCard";
import type { ToastInstance } from "components/ToastCard";
import useFeatureFlags from "hooks/useFeatureFlags";
import {
getConfig,
getLoginError,
Expand All @@ -26,6 +27,7 @@ import UserPassForm from "./UserPassForm";
import { ErrorResponse, Label, TestId } from "./types";

export default function LogIn() {
useFeatureFlags();
const viewedAuthRequests = useRef<string[]>([]);
const config = useSelector(getConfig);
const isJuju = useSelector(getIsJuju);
Expand Down
3 changes: 3 additions & 0 deletions src/consts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,6 @@ export const DATETIME_LOCAL = "yyyy-MM-dd'T'HH:mm";
// This is set to 5 minutes as that is how long a token is valid for in JIMM, so
// if access is revoked this will poll and delete the cookie.
export const OIDC_POLL_INTERVAL = 5 * 60 * 1000;

// The local storage key for enabled feature flags list
export const ENABLED_FLAGS = "flags";
123 changes: 123 additions & 0 deletions src/hooks/useFeatureFlags.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
import { renderHook } from "@testing-library/react";

import useFeatureFlags from "./useFeatureFlags";
import * as LocalStorage from "./useLocalStorage";
import * as QueryParams from "./useQueryParams";

vi.mock("./useLocalStorage");
vi.mock("./useQueryParams");

describe("useFeatureFlags", () => {
beforeEach(() => {
vi.clearAllMocks();
});

it("should initialize with empty flags from local storage", () => {
vi.spyOn(LocalStorage, "default").mockReturnValue([[], vi.fn()]);
vi.spyOn(QueryParams, "useQueryParams").mockReturnValue([
{ "enable-flag": null, "disable-flag": null },
vi.fn(),
]);

renderHook(() => useFeatureFlags());

expect(LocalStorage.default).toHaveBeenCalledWith("flags", []);
});

it("should enable flags from query parameters", () => {
const setLocalStorage = vi.fn();
vi.spyOn(LocalStorage, "default").mockReturnValue([[], setLocalStorage]);
vi.spyOn(QueryParams, "useQueryParams").mockReturnValue([
{ "enable-flag": ["featureA", "featureB"], "disable-flag": null },
vi.fn(),
]);

renderHook(() => useFeatureFlags());

expect(setLocalStorage).toHaveBeenCalledWith(["featureA", "featureB"]);
});

it("should disable flags from query parameters", () => {
const setLocalStorage = vi.fn();
vi.spyOn(LocalStorage, "default").mockReturnValue([
["featureA", "featureB", "featureC"],
setLocalStorage,
]);
vi.spyOn(QueryParams, "useQueryParams").mockReturnValue([
{ "enable-flag": null, "disable-flag": ["featureB"] },
vi.fn(),
]);

renderHook(() => useFeatureFlags());

expect(setLocalStorage).toHaveBeenCalledWith(["featureA", "featureC"]);
});

it("should handle both enable and disable flags", () => {
const setLocalStorage = vi.fn();
vi.spyOn(LocalStorage, "default").mockReturnValue([
["featureA", "featureB", "featureC"],
setLocalStorage,
]);
vi.spyOn(QueryParams, "useQueryParams").mockReturnValue([
{ "enable-flag": ["featureD"], "disable-flag": ["featureB"] },
vi.fn(),
]);

renderHook(() => useFeatureFlags());

expect(setLocalStorage).toHaveBeenCalledWith([
"featureA",
"featureC",
"featureD",
]);
});

it("should not update local storage when flags are the same", () => {
const setLocalStorage = vi.fn();
vi.spyOn(LocalStorage, "default").mockReturnValue([
["featureA", "featureB"],
setLocalStorage,
]);
vi.spyOn(QueryParams, "useQueryParams").mockReturnValue([
{ "enable-flag": ["featureA", "featureB"], "disable-flag": null },
vi.fn(),
]);

renderHook(() => useFeatureFlags());

expect(setLocalStorage).not.toHaveBeenCalledTimes(2);
});

it("should handle null query params", () => {
const setLocalStorage = vi.fn();
vi.spyOn(LocalStorage, "default").mockReturnValue([
["featureA"],
setLocalStorage,
]);
vi.spyOn(QueryParams, "useQueryParams").mockReturnValue([
{ "enable-flag": null, "disable-flag": null },
vi.fn(),
]);

renderHook(() => useFeatureFlags());

expect(setLocalStorage).not.toHaveBeenCalled();
});

it("should handle empty array query params", () => {
const setLocalStorage = vi.fn();
vi.spyOn(LocalStorage, "default").mockReturnValue([
["featureA"],
setLocalStorage,
]);
vi.spyOn(QueryParams, "useQueryParams").mockReturnValue([
{ "enable-flag": [], "disable-flag": [] },
vi.fn(),
]);

renderHook(() => useFeatureFlags());

expect(setLocalStorage).not.toHaveBeenCalled();
});
});
48 changes: 48 additions & 0 deletions src/hooks/useFeatureFlags.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { useCallback, useEffect, useMemo } from "react";

import { ENABLED_FLAGS } from "consts";

import useLocalStorage from "./useLocalStorage";
import { useQueryParams } from "./useQueryParams";

type FlagQueryParams = {
"enable-flag": string[] | null;
"disable-flag": string[] | null;
};

export default function useFeatureFlags() {
const [queryParams] = useQueryParams<FlagQueryParams>({
"enable-flag": [],
"disable-flag": [],
});
const [enabledFlags, setEnabledFlags] = useLocalStorage<string[]>(
ENABLED_FLAGS,
[],
);

const getFinalFlags = useCallback(() => {
const enabledParams = queryParams["enable-flag"] ?? [];
const disabledParams = queryParams["disable-flag"] ?? [];

// remove disabled params from flag list
let result = enabledFlags.filter((flag) => !disabledParams.includes(flag));

// append enabled params to flag list
result = enabledParams.reduce((acc, param) => {
if (!disabledParams.includes(param) && !acc.includes(param)) {
return [...acc, param];
}
return acc;
}, result);
return result;
}, [queryParams, enabledFlags]);

const finalFlags = useMemo(() => getFinalFlags(), [getFinalFlags]);

useEffect(() => {
// deep comparison to prevent unnecessary re-renders
if (JSON.stringify(finalFlags) !== JSON.stringify(enabledFlags)) {
setEnabledFlags(finalFlags);
}
}, [setEnabledFlags, finalFlags, enabledFlags]);
}
36 changes: 35 additions & 1 deletion src/store/middleware/model-poller.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ describe("model poller", () => {
});

beforeEach(() => {
vi.clearAllMocks();
vi.useFakeTimers();
next = vi.fn();
fakeStore = {
Expand Down Expand Up @@ -129,6 +130,7 @@ describe("model poller", () => {

afterEach(() => {
vi.restoreAllMocks();
localStorage.clear();
vi.useRealTimers();
});

Expand Down Expand Up @@ -264,6 +266,7 @@ describe("model poller", () => {
});

it("disables the controller features if JIMM < 4", async () => {
localStorage.setItem("flags", JSON.stringify(["rebac"]));
conn.facades.modelManager.listModels.mockResolvedValue({
"user-models": [],
});
Expand Down Expand Up @@ -292,7 +295,38 @@ describe("model poller", () => {
);
});

it("updates the controller features if JIMM >= 4", async () => {
it("disables rebac features if JIMM >= 4 but feature flag is disabled", async () => {
localStorage.setItem("flags", JSON.stringify([]));
conn.facades.modelManager.listModels.mockResolvedValue({
"user-models": [],
});
conn.facades.jimM = {
checkRelation: vi.fn().mockImplementation(async () => ({
allowed: true,
})),
version: 4,
};
vi.spyOn(jujuModule, "loginWithBakery").mockImplementation(async () => ({
conn,
intervalId,
juju,
}));
await runMiddleware();
expect(next).not.toHaveBeenCalled();
expect(fakeStore.dispatch).toHaveBeenCalledWith(
generalActions.updateControllerFeatures({
wsControllerURL,
features: {
auditLogs: true,
crossModelQueries: true,
rebac: false,
},
}),
);
});

it("updates the controller features if JIMM >= 4 and feature flag enabled", async () => {
localStorage.setItem("flags", JSON.stringify(["rebac"]));
conn.facades.modelManager.listModels.mockResolvedValue({
"user-models": [],
});
Expand Down
5 changes: 4 additions & 1 deletion src/store/middleware/model-poller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,16 @@ export const modelPollerMiddleware: Middleware<
}),
);
const jimmVersion = conn.facades.jimM?.version ?? 0;
const rebacFlagEnabled = JSON.parse(
window.localStorage.getItem("flags") ?? "[]",
).includes("rebac");
reduxStore.dispatch(
generalActions.updateControllerFeatures({
wsControllerURL,
features: {
auditLogs: jimmVersion >= 4,
crossModelQueries: jimmVersion >= 4,
rebac: jimmVersion >= 4,
rebac: rebacFlagEnabled && jimmVersion >= 4,
},
}),
);
Expand Down

0 comments on commit 32d9e8f

Please sign in to comment.