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

feat: Implement rebac feature flag via query params #1871

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
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