Conversation
✅ Deploy Preview for tradetrust-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for reference-implementation ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughThree distinct changes: a helper function for API headers introduced in storageAPI, environment URL replaced with hardcoded local endpoint in useQueue hook, and a comprehensive test flow overhaul for Magic Link integration including device registration, verification code handling, and wallet operations. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/common/API/storageAPI.tsx (1)
28-45: Move API key handling to server-side function; apply provided typing fixThe security concern is valid: storageAPI.tsx is imported and used directly from useQueue.tsx (a client-side React hook), meaning process.env.API_KEY_DOCUMENT_STORAGE is bundled into browser code and sent in request headers. This exposes credentials in the client bundle and network traffic even in development.
Since calls already route to a serverless function (
http://localhost:5080/.netlify/functions/storage), move the API key to the server-side function—client should omit the x-api-key header and rely on withCredentials + CSRF flow alone.The typing refactor in the diff is correct and should be applied:
-const headers: any = { +const headers: AxiosRequestHeaders = {Add axios timeouts to
getQueueNumberanduploadToStoragecalls (e.g.,timeout: 10000) to prevent hanging.
🧹 Nitpick comments (6)
src/common/API/storageAPI.tsx (2)
14-15: Confirm CSRF token endpoint requires API key; otherwise omit to reduce exposureIf /csrf-token does not require authentication, avoid sending x-api-key here. It reduces key exposure surface.
47-55: Consider credentials/timeout on queue callIf the queue endpoint sets cookies for CSRF, add withCredentials: true and a timeout to prevent hangs under load.
return axios({ method: "get", url: url, - headers: getHeaders(), + headers: getHeaders(), + // withCredentials: true, // enable if cookie is expected here + timeout: 10000, });src/test/creator/happy-flow-magiclink.spec.ts (4)
80-110: Replace fixed waits with expectations to reduce flakinessPrefer waiting on selectors with timeouts over arbitrary t.wait calls.
- await t.wait(2000); // Increased wait for iframe loading - await validateMagicIframeSelector(Selector("p").withText("Sign in to")); + await t.expect(Selector(".magic-iframe").exists).ok({ timeout: 15000 }); + await validateMagicIframeSelector(Selector("p").withText("Sign in to")); - await clickMagicIframeButton(Selector('button[aria-label="Email"]')); - await t.wait(3000); // Wait for email input form to load + await clickMagicIframeButton(Selector('button[aria-label="Email"]')); + await t.expect(emailInput.exists).ok({ timeout: 10000 });
118-156: Clean up device-registration email to keep inbox tidyAfter successful approval, delete the registration email like you already do for the verification email.
- console.log("✅ Device registration approved"); + console.log("✅ Device registration approved"); + if (registrationEmail?.id) { + await mailslurp.deleteEmail(registrationEmail.id); + }
183-186: Trim wallet address text before useinnerText can include whitespace. Trim to avoid hidden char bugs.
- const walletAddress = await walletAddressDiv.innerText; + const walletAddress = (await walletAddressDiv.innerText).trim();
250-252: Assert contains instead of exact equals for success titleUI copy can vary; use contains to reduce brittleness.
- await t.expect(processTitle.innerText).eql("Document issued successfully"); + await t.expect(processTitle.innerText).contains("Document issued successfully");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/common/API/storageAPI.tsx(2 hunks)src/common/hooks/useQueue.tsx(1 hunks)src/test/creator/happy-flow-magiclink.spec.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/test/creator/happy-flow-magiclink.spec.ts (2)
src/test/helper.js (10)
navigateToCreator(39-48)navigateToCreator(39-48)validateMagicIframeSelector(61-65)validateMagicIframeSelector(61-65)clickMagicIframeButton(73-77)clickMagicIframeButton(73-77)inputMagicIframeTexts(67-71)inputMagicIframeTexts(67-71)location(86-86)location(86-86)src/test/setup-contracts-testcafe.js (2)
provider(38-38)signer(39-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: Redirect rules - tradetrust-dev
- GitHub Check: Redirect rules - reference-implementation
- GitHub Check: Header rules - reference-implementation
- GitHub Check: Header rules - tradetrust-dev
- GitHub Check: Pages changed - reference-implementation
- GitHub Check: Pages changed - tradetrust-dev
- GitHub Check: Lighthouse Desktop
- GitHub Check: Synpress e2e Test
- GitHub Check: Lint & Test
- GitHub Check: Testcafe e2e Test
🔇 Additional comments (1)
src/test/creator/happy-flow-magiclink.spec.ts (1)
16-17: LGTM: dotenv initLoading env early is good; keeps tests configurable.
| setQueueState(QueueState.PENDING); | ||
|
|
||
| const documentStorageURL = process.env.DOCUMENT_STORAGE_URL; | ||
| const documentStorageURL = "http://localhost:5080/.netlify/functions/storage"; //process.env.DOCUMENT_STORAGE_URL; |
There was a problem hiding this comment.
🧩 Analysis chain
Do not hardcode storage URL; gate to preview/local or revert before merge
Merging this will force all environments to localhost. Restore env-based config with a local fallback for dev/test only.
Apply this minimal fix:
- const documentStorageURL = "http://localhost:5080/.netlify/functions/storage"; //process.env.DOCUMENT_STORAGE_URL;
+ const documentStorageURL =
+ process.env.DOCUMENT_STORAGE_URL ||
+ (window.location.hostname === "localhost"
+ ? "http://localhost:5080/.netlify/functions/storage"
+ : "");Optionally, fail gracefully in non-local envs by skipping QR/storage when the URL is empty instead of throwing.
Run locally and on preview to confirm:
- Local: QR shows and uploads hit localhost storage.
- Preview/Prod: reads DOCUMENT_STORAGE_URL and never points to localhost.
I'll verify the review comment by examining the actual code and checking the codebase for context.
🏁 Script executed:
#!/bin/bash
# Check the actual code at line 133 in useQueue.tsx
fd -e tsx -e ts 'useQueue' | head -5Length of output: 29
🏁 Script executed:
#!/bin/bash
# Search for the file and inspect around line 133
find . -name "useQueue.tsx" -type f 2>/dev/null | head -1 | xargs -I {} sh -c 'cat -n {} | sed -n "125,140p"'Length of output: 624
🏁 Script executed:
#!/bin/bash
# Search for DOCUMENT_STORAGE_URL references throughout codebase
rg -n 'DOCUMENT_STORAGE_URL|documentStorageURL' --type=ts --type=tsx -C 3Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check for environment configuration files
fd -e 'env' -e 'env.local' -e 'env.example' | head -10Length of output: 0
🏁 Script executed:
#!/bin/bash
# Search for localhost:5080 to understand scope of hardcoding
rg -n 'localhost:5080' --type=ts --type=tsx --type=json -C 2Length of output: 27
🏁 Script executed:
#!/bin/bash
# Check environment setup in package.json or build config
find . -maxdepth 2 -name "package.json" -o -name "*.config.*" | xargs grep -l "DOCUMENT_STORAGE_URL" 2>/dev/null || echo "No env config found"Length of output: 19
🏁 Script executed:
#!/bin/bash
# Search for DOCUMENT_STORAGE_URL without type flags
rg -n 'DOCUMENT_STORAGE_URL' -C 3Length of output: 503
🏁 Script executed:
#!/bin/bash
# Search for localhost:5080 without type flags
rg -n 'localhost:5080' -C 2Length of output: 373
🏁 Script executed:
#!/bin/bash
# Check the entire function context around line 133
cat -n src/common/hooks/useQueue.tsx | sed -n "120,160p"Length of output: 2017
🏁 Script executed:
#!/bin/bash
# Look for env setup in CI/deployment files
find . -maxdepth 3 \( -name ".github" -o -name ".gitlab-ci*" -o -name "netlify*" -o -name "*workflow*" \) -type d 2>/dev/null | head -5Length of output: 29
🏁 Script executed:
#!/bin/bash
# Search for any env variable pattern setup
rg -n 'process\.env\.' -C 2 src/common/hooks/useQueue.tsxLength of output: 489
🏁 Script executed:
#!/bin/bash
# Check if there are deployment or env config files at root
ls -la | grep -E 'env|netlify|deploy|config'Length of output: 963
🏁 Script executed:
#!/bin/bash
# Check the content of env files to see if DOCUMENT_STORAGE_URL is defined
echo "=== .env.sample ===" && cat .env.sample && echo -e "\n=== .env.development ===" && cat .env.development && echo -e "\n=== .env.production ===" && cat .env.production && echo -e "\n=== .env.test ===" && cat .env.testLength of output: 977
🏁 Script executed:
#!/bin/bash
# Check netlify.toml for environment variable configuration
cat netlify.tomlLength of output: 583
🏁 Script executed:
#!/bin/bash
# Check GitHub workflows for environment setup
find .github/workflows -name "*.yml" -o -name "*.yaml" | head -3 | xargs -I {} sh -c 'echo "=== {} ===" && cat {}'Length of output: 3295
🏁 Script executed:
#!/bin/bash
# Check how documentStorageURL is actually used in the function
cat -n src/common/hooks/useQueue.tsx | sed -n "160,200p"Length of output: 1779
Hardcoded localhost URL will break production deployments; restore environment-based config
Line 133 hardcodes documentStorageURL to localhost, but the required environment variable DOCUMENT_STORAGE_URL is documented in .env.sample and never initialized in .env files or GitHub workflows. In production, the code will either throw an error at line 168 or attempt to reach an inaccessible localhost endpoint.
- const documentStorageURL = "http://localhost:5080/.netlify/functions/storage"; //process.env.DOCUMENT_STORAGE_URL;
+ const documentStorageURL =
+ process.env.DOCUMENT_STORAGE_URL ||
+ (window.location.hostname === "localhost"
+ ? "http://localhost:5080/.netlify/functions/storage"
+ : "");Verify locally that QR generation and storage upload work, and confirm that staging/production use the correct DOCUMENT_STORAGE_URL from environment variables.
🤖 Prompt for AI Agents
In src/common/hooks/useQueue.tsx around line 133 the documentStorageURL is
hardcoded to "http://localhost:5080/.netlify/functions/storage"; restore using
the environment variable DOCUMENT_STORAGE_URL (process.env.DOCUMENT_STORAGE_URL)
instead, add a runtime check that throws or logs a clear error if the variable
is missing, and ensure local .env, staging and production GitHub workflow/env
configurations are updated to provide the correct URL; after making the change,
verify locally that QR generation and storage upload work against the configured
endpoint and confirm staging/production pick up the env var.
| const email = await mailslurp.waitForLatestEmail(inbox.id, 30000, true); | ||
| // console.log("📧 Verification email received"); | ||
| // use regex to extract the confirmation code which is 6 digits | ||
| const code = /[^#]([0-9]{6})/.exec(email!.body!)?.[1]; | ||
|
|
||
| if (!code) { | ||
| console.error("❌ Failed to extract verification code from email:", email.body); | ||
| throw new Error("Verification code not found in email"); | ||
| } | ||
|
|
There was a problem hiding this comment.
Make verification-code regex robust
The current /^#/ fails at string start and is brittle. Use word boundaries.
- const code = /[^#]([0-9]{6})/.exec(email!.body!)?.[1];
+ const code = email?.body?.match(/\b(\d{6})\b/)?.[1];Also consider trimming and normalizing email body if HTML.
🤖 Prompt for AI Agents
In src/test/creator/happy-flow-magiclink.spec.ts around lines 160-169, the
current regex /[^#]([0-9]{6})/ is brittle and fails at string start; replace it
with a word-boundary based pattern such as /\b(\d{6})\b/ to reliably capture a
6-digit code, and before applying the regex normalize the email body by trimming
and converting HTML emails to plain text (strip tags or use the email's
textContent) and collapse whitespace so the code can be consistently matched.
| // console.log("💸 Transferring funds to Magic wallet..."); | ||
| try { | ||
| const wallet = new ethers.Wallet("0xe82294532bcfcd8e0763ee5cef194f36f00396be59b94fb418f5f8d83140d9a7"); | ||
| const provider = new ethers.providers.JsonRpcProvider("http://localhost:8545"); | ||
| const signer = wallet.connect(provider); | ||
|
|
||
| const balance = await signer.getBalance(); | ||
| // console.log(`💰 Funder wallet balance: ${ethers.utils.formatEther(balance)} ETH`); | ||
|
|
||
| if (!balance.gt(BigNumber.from("10000000000000000000"))) { | ||
| throw new Error(`Insufficient balance: ${ethers.utils.formatEther(balance)} ETH`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Avoid committing a hardcoded private key; read from env
Use an env var for the funder key; fail with a clear error if missing.
- const wallet = new ethers.Wallet("0xe82294532bcfcd8e0763ee5cef194f36f00396be59b94fb418f5f8d83140d9a7");
+ const FUNDER_PRIVATE_KEY = process.env.FUNDER_PRIVATE_KEY;
+ if (!FUNDER_PRIVATE_KEY) {
+ throw new Error("Missing FUNDER_PRIVATE_KEY for test funding");
+ }
+ const wallet = new ethers.Wallet(FUNDER_PRIVATE_KEY);Ensure .env/.env.ci define FUNDER_PRIVATE_KEY for CI runs.
I'll verify the review comment by examining the codebase. Let me check if the hardcoded private key exists, current environment variable handling, and the state of .env files.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check if file exists and show the problematic code section
if [ -f "src/test/creator/happy-flow-magiclink.spec.ts" ]; then
echo "=== File found. Showing lines 185-205 ==="
sed -n '185,205p' "src/test/creator/happy-flow-magiclink.spec.ts"
else
echo "File not found"
fi
Length of output: 888
🏁 Script executed:
#!/bin/bash
# Search for FUNDER_PRIVATE_KEY usage across the codebase
echo "=== Searching for FUNDER_PRIVATE_KEY ==="
rg -n "FUNDER_PRIVATE_KEY" --type ts --type js
echo ""
echo "=== Searching for hardcoded private keys (0x pattern with 64 hex chars) ==="
rg -nP '0x[a-fA-F0-9]{64}' --type ts --type js src/test/Length of output: 4298
🏁 Script executed:
#!/bin/bash
# Check for .env and .env.ci files
echo "=== Checking for .env files ==="
find . -maxdepth 2 -name ".env*" -type f | head -20
echo ""
echo "=== Checking for .env.example or similar ==="
fd -e "env" -e "example" . -H | grep -i env | head -20Length of output: 140
🏁 Script executed:
#!/bin/bash
# Check if process.env is used for secrets in test setup
echo "=== Checking for process.env usage in test files ==="
rg -n "process\.env" src/test/ --type ts --type js -C 2 | head -50Length of output: 561
Let me check the .env files to see if FUNDER_PRIVATE_KEY is already defined there:
---
🏁 Script executed:
```shell
#!/bin/bash
# Check contents of .env files
echo "=== .env.test ==="
cat .env.test
echo ""
echo "=== .env.sample ==="
cat .env.sample
echo ""
echo "=== .env.development ==="
cat .env.development
Length of output: 921
🏁 Script executed:
#!/bin/bash
# Check if there's any documentation or comments about the hardcoded key
echo "=== Context around the hardcoded key in happy-flow-magiclink.spec.ts ==="
sed -n '180,210p' "src/test/creator/happy-flow-magiclink.spec.ts"
echo ""
echo "=== Check if this is a test-only file ==="
head -20 "src/test/creator/happy-flow-magiclink.spec.ts"Length of output: 1902
Remove hardcoded private key and read from environment variable
The hardcoded key at line 190 is confirmed. Move it to process.env.FUNDER_PRIVATE_KEY with validation, matching the pattern already used in this file for MAILSLURP_API_KEY:
- const wallet = new ethers.Wallet("0xe82294532bcfcd8e0763ee5cef194f36f00396be59b94fb418f5f8d83140d9a7");
+ const FUNDER_PRIVATE_KEY = process.env.FUNDER_PRIVATE_KEY;
+ if (!FUNDER_PRIVATE_KEY) {
+ throw new Error("Missing FUNDER_PRIVATE_KEY for test funding");
+ }
+ const wallet = new ethers.Wallet(FUNDER_PRIVATE_KEY);Add FUNDER_PRIVATE_KEY= to .env.sample and define it in .env.test for local test runs and in CI environment variables for CI runs.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // console.log("💸 Transferring funds to Magic wallet..."); | |
| try { | |
| const wallet = new ethers.Wallet("0xe82294532bcfcd8e0763ee5cef194f36f00396be59b94fb418f5f8d83140d9a7"); | |
| const provider = new ethers.providers.JsonRpcProvider("http://localhost:8545"); | |
| const signer = wallet.connect(provider); | |
| const balance = await signer.getBalance(); | |
| // console.log(`💰 Funder wallet balance: ${ethers.utils.formatEther(balance)} ETH`); | |
| if (!balance.gt(BigNumber.from("10000000000000000000"))) { | |
| throw new Error(`Insufficient balance: ${ethers.utils.formatEther(balance)} ETH`); | |
| } | |
| // console.log("💸 Transferring funds to Magic wallet..."); | |
| try { | |
| const FUNDER_PRIVATE_KEY = process.env.FUNDER_PRIVATE_KEY; | |
| if (!FUNDER_PRIVATE_KEY) { | |
| throw new Error("Missing FUNDER_PRIVATE_KEY for test funding"); | |
| } | |
| const wallet = new ethers.Wallet(FUNDER_PRIVATE_KEY); | |
| const provider = new ethers.providers.JsonRpcProvider("http://localhost:8545"); | |
| const signer = wallet.connect(provider); | |
| const balance = await signer.getBalance(); | |
| // console.log(`💰 Funder wallet balance: ${ethers.utils.formatEther(balance)} ETH`); | |
| if (!balance.gt(BigNumber.from("10000000000000000000"))) { | |
| throw new Error(`Insufficient balance: ${ethers.utils.formatEther(balance)} ETH`); | |
| } |
🤖 Prompt for AI Agents
In src/test/creator/happy-flow-magiclink.spec.ts around lines 188 to 199,
replace the hardcoded private key with process.env.FUNDER_PRIVATE_KEY and
validate it the same way MAILSLURP_API_KEY is validated: check the env var
exists, matches expected hex/private-key pattern (0x + 64 hex chars) and throw a
clear error if missing/invalid; use that value to create the ethers.Wallet and
keep the rest of the logic unchanged. Also add FUNDER_PRIVATE_KEY= to
.env.sample and set a test key in .env.test and ensure CI secrets include
FUNDER_PRIVATE_KEY.
1 participant
Summary
For testing purpose only on preview. Reject PR after testing
Summary by CodeRabbit
Tests
Chores