-
Notifications
You must be signed in to change notification settings - Fork 3
Adds base infra for the Unified Ecommerce frontend #1634
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
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
cf1bd4b
Bootstrapping the Unified Ecommerce frontend
jkachel 63d115e
Finishing this up; should work now (to the extent that it works)
jkachel fa8253a
adding tests
jkachel 956248e
Addressing comments in the PR
jkachel 3a02e22
fixing docs
jkachel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| // Feature flags for the app. These should correspond to the flag that's set up | ||
| // in PostHog. | ||
|
|
||
| export enum FeatureFlags { | ||
| EnableEcommerce = "enable-ecommerce", | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
33 changes: 33 additions & 0 deletions
33
frontends/mit-learn/src/page-components/EcommerceFeature/EcommerceFeature.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import React from "react" | ||
| import { useFeatureFlagEnabled } from "posthog-js/react" | ||
| import { ForbiddenError } from "@/common/permissions" | ||
| import { FeatureFlags } from "@/common/feature_flags" | ||
|
|
||
| type EcommerceFeatureProps = { | ||
| children: React.ReactNode | ||
| } | ||
|
|
||
| /** | ||
| * Simple wrapper to standardize the feature flag check for ecommerce UI pages. | ||
| * If the flag is enabled, display the children; if not, throw a ForbiddenError | ||
| * like you'd get for an unauthenticated route. | ||
| * | ||
| * There's a PostHogFeature component that is provided but went this route | ||
| * because it seemed to be inconsistent - sometimes having the flag enabled | ||
| * resulted in it tossing to the error page. | ||
| * | ||
| * Set the feature flag here using the enum, and then make sure it's also | ||
| * defined in commmon/feature_flags too. | ||
| */ | ||
|
|
||
| const EcommerceFeature: React.FC<EcommerceFeatureProps> = ({ children }) => { | ||
| const ecommFlag = useFeatureFlagEnabled(FeatureFlags.EnableEcommerce) | ||
|
|
||
| if (ecommFlag === false) { | ||
| throw new ForbiddenError("Not enabled.") | ||
| } | ||
|
|
||
| return ecommFlag ? children : null | ||
| } | ||
|
|
||
| export default EcommerceFeature | ||
65 changes: 65 additions & 0 deletions
65
frontends/mit-learn/src/pages/EcommercePages/CartPage.test.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| import { renderTestApp, waitFor, setMockResponse } from "../../test-utils" | ||
| import { urls } from "api/test-utils" | ||
| import * as commonUrls from "@/common/urls" | ||
| import { Permissions } from "@/common/permissions" | ||
| import { login } from "@/common/urls" | ||
| import { useFeatureFlagEnabled } from "posthog-js/react" | ||
|
|
||
| jest.mock("posthog-js/react") | ||
| const mockedUseFeatureFlagEnabled = jest.mocked(useFeatureFlagEnabled) | ||
|
|
||
| const oldWindowLocation = window.location | ||
|
|
||
| beforeAll(() => { | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| delete (window as any).location | ||
|
|
||
| window.location = Object.defineProperties({} as Location, { | ||
| ...Object.getOwnPropertyDescriptors(oldWindowLocation), | ||
| assign: { | ||
| configurable: true, | ||
| value: jest.fn(), | ||
| }, | ||
| }) | ||
| }) | ||
|
|
||
| afterAll(() => { | ||
| window.location = oldWindowLocation | ||
| }) | ||
|
|
||
| describe("CartPage", () => { | ||
| ;["on", "off"].forEach((testCase: string) => { | ||
| test(`Renders when logged in and feature flag is ${testCase}`, async () => { | ||
| setMockResponse.get(urls.userMe.get(), { | ||
| [Permissions.Authenticated]: true, | ||
| }) | ||
| mockedUseFeatureFlagEnabled.mockReturnValue(testCase === "on") | ||
|
|
||
| renderTestApp({ | ||
| url: commonUrls.ECOMMERCE_CART, | ||
| }) | ||
| await waitFor(() => { | ||
| testCase === "on" | ||
| ? expect(document.title).toBe("Shopping Cart | MIT Learn") | ||
| : expect(document.title).not.toBe("Shopping Cart | MIT Learn") | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| test("Sends to login page when logged out", async () => { | ||
| setMockResponse.get(urls.userMe.get(), { | ||
| [Permissions.Authenticated]: false, | ||
| }) | ||
| const expectedUrl = login({ | ||
| pathname: "/cart/", | ||
| }) | ||
|
|
||
| renderTestApp({ | ||
| url: commonUrls.ECOMMERCE_CART, | ||
| }) | ||
|
|
||
| await waitFor(() => { | ||
| expect(window.location.assign).toHaveBeenCalledWith(expectedUrl) | ||
| }) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import React from "react" | ||
| import { Breadcrumbs, Container, Typography } from "ol-components" | ||
| import EcommerceFeature from "@/page-components/EcommerceFeature/EcommerceFeature" | ||
| import MetaTags from "@/page-components/MetaTags/MetaTags" | ||
| import * as urls from "@/common/urls" | ||
|
|
||
| const CartPage: React.FC = () => { | ||
| return ( | ||
| <EcommerceFeature> | ||
| <Container> | ||
| <MetaTags title="Shopping Cart" /> | ||
| <Breadcrumbs | ||
| variant="light" | ||
| ancestors={[{ href: urls.HOME, label: "Home" }]} | ||
| current="Shopping Cart" | ||
| /> | ||
|
|
||
| <Typography component="h1" variant="h3"> | ||
| Shopping Cart | ||
| </Typography> | ||
|
|
||
| <Typography> | ||
| The shopping cart layout should go here, if you're allowed to see | ||
| this. | ||
| </Typography> | ||
| </Container> | ||
| </EcommerceFeature> | ||
| ) | ||
| } | ||
|
|
||
| export default CartPage |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| # Unified Ecommerce in MIT Learn | ||
|
|
||
| The front end for the Unified Ecommerce system lives in MIT Learn. So, pages that exist here are designed to talk to Unified Ecommerce rather than to the Learn system. | ||
|
|
||
| There's a few functional pieces here: | ||
|
|
||
| - **Cart** - Displays the user's cart, and provides some additional functionality for that (item management, discount application, etc.) | ||
| - **Receipts** - Allows the user to display their order history and view receipts from their purchases, including historical ones from other systems. | ||
| - **Financial Assistance** - For learning resources that support it, the learner side of the financial assistance request system lives here. (Approvals do not.) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Post-merge comment
I noticed some odd behavior with PostHog and feature flags that I thought was worth mentioning. Essentially, I think there's no reliable way—with
useFeatureFlagEnabled—to determine whether a flag is "off" or "not yet loaded". (Note: theuseFeatureFlagEnabledis really simple. Here's its source.)useFeatureFlagEnabledreturnsboolean | undefined. Its behavior is:useFeatureFlagEnabled("flag-A")returns undefined before any flags have loadeduseFeatureFlagEnabled("flag-A")returnstrueif the flag is on, andfalseif the flag is off or has not loaded yet.So the following is possible:
In this scenario, we were attempting to:
But this doesn't seem to work quite right if any flags are bootstrapped, as shown in the sreenshot.