Skip to content

Commit

Permalink
comment cleanup, simplify login button, and add one more test
Browse files Browse the repository at this point in the history
  • Loading branch information
doug-s-nava committed Jan 2, 2025
1 parent 971a129 commit 3988586
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 31 deletions.
17 changes: 1 addition & 16 deletions frontend/src/components/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,6 @@ type Props = {

const homeRegexp = /^\/(?:e[ns])?$/;

const LoginLink = () => {
return (
<Link
href=""
className="usa-nav__link text-primary font-sans-2xs display-flex text-normal"
>
<USWDSIcon
className="usa-icon margin-right-05 margin-left-neg-05"
name="login"
/>
Login
</Link>
);
};

const NavLinks = ({
mobileExpanded,
onToggleMobileNav,
Expand Down Expand Up @@ -204,7 +189,7 @@ const Header = ({ logoPath, locale }: Props) => {
label={t("nav_menu_toggle")}
/>
</div>
{!hideLoginLink && <LoginLink />}
{!hideLoginLink && <span>Auth Enabled</span>}
<NavLinks
mobileExpanded={isMobileNavExpanded}
onToggleMobileNav={handleMobileNavToggle}
Expand Down
1 change: 1 addition & 0 deletions frontend/src/hooks/useFeatureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export function useFeatureFlags(): {
setFeatureFlags(flagsFromCookie);
}, []);

// Note that values set in cookies will be persistent per browser session unless explicitly overwritten
const setFeatureFlag = useCallback(
(flagName: string, value: boolean) => {
const newFlags = {
Expand Down
42 changes: 27 additions & 15 deletions frontend/src/services/featureFlags/FeatureFlagManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { NextRequest, NextResponse } from "next/server";
* - middleware: for use in setting feature flags on cookies in next middleware
* - isFeatureEnabled: for checking if a feature is currently enabled, requires
* passing in existing request cookies
*
*/
export class FeatureFlagsManager {
// Define all feature flags here! You can override these in your
Expand All @@ -51,13 +52,11 @@ export class FeatureFlagsManager {

/*
- Flags set by environment variables are the first override to default values
* - Flags set by environment variables are the first override to default values
- Flags set in the /dev/feature-flags admin view will be set in the cookie
- Flags set in query params will result in the flag value being stored in the cookie
- As query param values are set in middleware on each request, query params have the highest precedence
- Values set in cookies will be persistent per browser session unless explicitly overwritten
- This means that simply removing a query param from a url will not revert the feature flag value to
the value specified in environment variable or default, you'll need to clear cookies or open a new private browser
*/
get featureFlags(): FeatureFlags {
Expand All @@ -69,7 +68,17 @@ export class FeatureFlagsManager {

/**
* Check whether a feature flag is enabled
*
* @description Server side flag values are interpreted as follows:
*
* - Default values and any environment variable overrides are grabbed from the class
* - Any flags from the passed in request cookie will override the class's stored flags
* - Any flags set in the query param are a final override, though these likely will have already been set on the cookie
* by the middleware
*
* @param name - Feature flag name
* @param cookies - server side cookies to check for enabled flags
* @param searchParams - search params to check for enabled flags
* @example isFeatureEnabled("featureFlagName")
*/
isFeatureEnabled(
Expand Down Expand Up @@ -98,35 +107,38 @@ export class FeatureFlagsManager {
}

/**
* Load feature flags from query params and set them on the cookie. This allows for
* Load feature flags from class, existing cookie, and query params and set them on the response cookie. This allows for
* feature flags to be set via url query params as well.
*
* Expects a query string with a param of `FEATURE_FLAGS_KEY`.
*
* For example, `example.com?_ff=showSite:true;enableClaimFlow:false`
* Expects a query string with a param of `_ff`. For example, `example.com?_ff=showSite:true;enableClaimFlow:false`
* would enable `showSite` and disable `enableClaimFlow`.
*
* Note that since flags set in a query param are persisted into the cookie, simply removing a query param from a url
* will not revert the feature flag value to the value specified in environment variable or default.
* To reset a flag value you'll need to clear cookies or open a new private browser
*
* This functionality allows environment variable based feature flag values, which otherwise cannot be made
* available to the client in our current setup, to be exposed to the client via the cookie.
*
*/
middleware(request: NextRequest, response: NextResponse): NextResponse {
// handle query param
const paramValue = request.nextUrl.searchParams.get(FEATURE_FLAGS_KEY);

const featureFlagsFromQuery =
paramValue === "reset"
? this.defaultFeatureFlags
? this.featureFlags
: parseFeatureFlagsFromString(paramValue);

// previously there was logic here to break if there were no feature flags active
// beyond default values. Unfortunately, this breaks the implementation of the dev admin
// view on the front end. Easier to just always set the cookie
// previously there was logic here to return early if there were no feature flags active
// beyond default values. Unfortunately, this breaks the implementation of the feature
// flag admin view, which depends on reading all flags from cookies, so the logic has beeen removed

// create new cookie value based on calculated feature flags
const featureFlags = {
...this.featureFlags,
...getFeatureFlagsFromCookie(request.cookies),
...featureFlagsFromQuery,
};

// set new cookie on response
setCookie(JSON.stringify(featureFlags), response.cookies);

return response;
Expand Down
23 changes: 23 additions & 0 deletions frontend/tests/services/featureFlags/FeatureFlagManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,29 @@ describe("FeatureFlagsManager", () => {
}),
});
});

test("resets cookie to server side settings on `reset` value", () => {
const request = new NextRequest(
new Request("fakeUrl://not.real?_ff=reset"),
{},
);
const mockSet = jest.fn();
jest
.spyOn(NextResponse.prototype, "cookies", "get")
.mockReturnValue({ set: mockSet } as object as NextResponse["cookies"]);

const featureFlagsManager = new FeatureFlagsManager({ feature3: false });
featureFlagsManager.middleware(request, NextResponse.next());
expect(mockSet).toHaveBeenCalledWith({
expires: expect.any(Date) as jest.Expect,
name: FEATURE_FLAGS_KEY,
value: JSON.stringify({
feature1: true, // from default
feature2: false, // from default
feature3: false, // from env var
}),
});
});
});

describe("isFeatureEnabled", () => {
Expand Down

0 comments on commit 3988586

Please sign in to comment.