[ENHANCEMENT] [MER-5672] Adaptive Lesson: RC I Exploration — Decoding the Mystery of Greenhouse Molecules (Playwright)#6731
Conversation
…ult and SQL recipe
…N_API_KEY in CHANGELOG
…st email confirmation
AI Review — performanceStream private assets instead of buffering themfile: lib/oli_web/controllers/playwright_support_asset_controller.ex |
AI Review — securityUnbounded asset downloads can exhaust server resourcesfile: lib/oli/scenarios/playwright_asset_storage.ex |
AI Review — elixirNo issues found |
Not addressing this — this endpoint only serves a single local Playwright run at a time (gated behind enable_playwright_scenarios, off in prod, plus the scenario token) — there's no realistic concurrent-download scenario to cause memory pressure. The objects are tens-of-MB course archives, well within what a single BEAM process buffers comfortably. Would reconsider if this endpoint ever gained real concurrent traffic. |
Not adding a size cap for now, for the same reason: this endpoint is unreachable without the scenario token and compiled out of prod, and the objects served are known course archives that developers seed into the bucket themselves — not arbitrary or attacker-controlled uploads. A cap makes sense if this endpoint ever serves less-controlled content. |
nicocirio
left a comment
There was a problem hiding this comment.
Nice work! I ran the test locally after following the env var setup described here, and it completed successfully.
Suggestion: I’m fine with approving this as-is, but I want to call out the tradeoff. We’re reusing the existing automation_setup flow for the full test seed path, which is pragmatic, but it also means every environment needs an admin-created API key exposed to the test runner as PLAYWRIGHT_AUTOMATION_API_KEY (created from /admin/api_keys, with automation_setup_enabled enabled).
It looks like there was another possible direction here: extending the existing Playwright scenario path with a custom hook to handle the archive-ingest/setup flow. I don’t think that exists as a native scenario directive today, so it likely would have required extra scenario infrastructure or at least a purpose-built hook. Reusing automation_setup is simpler in the short term, but it does add a per-environment bootstrap step compared to a dedicated server-side secret or a scenario-based setup path.
MER-5672
Summary
Adds the Playwright test for the adaptive lesson "Exploration: Decoding the Mystery of Greenhouse Molecules" (Unit 5, REAL CHEM I), which drives the lesson's 33+ screens through the happy path (correct answers only) against a real imported course. Since the course and its correct answers are private content (course IP), this PR also adds new infrastructure so the test can fetch them from a private bucket instead of requiring local files or presigned URLs.
Context
The project is a full course; the test needs to navigate to the correct page within it. The lesson has 33 screens total (the student may not hit every one).
All course content — the lesson title, search term, completion text, and every correct answer — lives in a private answers JSON that must never be committed to the repo (it's public). Both that JSON and the course zip are stored in a private bucket (MinIO in dev, S3 in other environments), and the server serves them through an authenticated endpoint, so no developer needs to copy files around by hand.
What changed
Private asset infrastructure for Playwright:
GET /test/assets/*pathendpoint, serving files from a dedicated bucket (Oli.Scenarios.PlaywrightAssetStorage), authenticated with the same scenario token already used by the Playwright scenario runner.OliWeb.PlaywrightAuth) with a constant-time comparison, reused by both endpoints.Security hardening of that infrastructure:
PLAYWRIGHT_ASSETS_BUCKET) nor the automation API key (PLAYWRIGHT_AUTOMATION_API_KEY) has a default value in code — both must be configured explicitly. This avoids, respectively, an S3 bucket-squatting risk (bucket names are globally unique) and having a public, guessable credential in the repo for an endpoint (/api/v1/automation_setup) that is reachable in every environment.CHANGELOG.mdupdated to document both new environment variables.Test stability, found by running it repeatedly:
Verification
mix format,tsc,eslint, andprettierall clean on the touched files.master(no conflicts).Running the test locally
1. Required environment variables (add to your oli.env — no defaults, the app won't start this feature without them):
PLAYWRIGHT_ASSETS_BUCKET: the name of the bucket in your local MinIO (or S3) where you'll upload the private assets. No default — pick any name, as long as it matches the bucket you actually create.PLAYWRIGHT_AUTOMATION_API_KEY: generate this once, logged in as admin, at /admin/api_keys — fill in a hint, click "create" (generates a random value), toggle automation_setup_enabled on for it. Never reuse a value that appears in the repo or in this PR — the endpoint it protects is reachable in every environment.2. Seed the bucket once. Inside the bucket, create a
mer-5672/folder with these two files:(ask the team for both files — they're private content, not in the repo).
Only the bucket name is configurable (via
PLAYWRIGHT_ASSETS_BUCKET) — themer-5672/folder and both file names are hardcoded in this test's spec, so they must match exactly. The answers JSON's structure is also specific to this test; a future test for a different lesson would define its own schema.3. Start the server with those env vars loaded, then run:
The test takes 5-7 minutes (33+ screens with server-side evaluation on each). If any of the env vars above are missing, the test is skipped with an explicit message indicating which one.