fix(web): enforce 5-min stream duration, add CreatePaymentStream test… - #587
Conversation
…s, inject mock wallet in Playwright Detailed Resolution for Issues Fundable-Protocol#399, Fundable-Protocol#464, and Fundable-Protocol#465: 1. Issue Fundable-Protocol#399: web(stream-validation): require minimum 5-minute duration for payment streams - What was done: Updated validateEndTime function in apps/web/src/lib/stream-validation.ts. - How it was done: Added durationToSeconds check against start time (start + 300 seconds). If durationSeconds < 300 or endTime < start + 300, validateEndTime returns "Stream duration must be at least 5 minutes" instead of permitting 1-second streams. 2. Issue Fundable-Protocol#464: web(CreatePaymentStream.test): add component render tests for stream creation wizard - What was done: Created unit test suite in apps/web/src/components/modules/payment-stream/CreatePaymentStream.test.tsx. - How it was done: Utilized Vitest and React Testing Library to test wizard component rendering, header & form field presence, sidebar stream summary section rendering, user form input changes, and action button layout. 3. Issue Fundable-Protocol#465: web(playwright.config): inject mock wallet provider in Playwright fixture - What was done: Configured Playwright fixture to inject mock wallet provider state in apps/web/playwright.config.ts. - How it was done: Added initScript under Playwright use configuration block to inject mock window.stellarWallet, window.freighterApi, and window.albedo objects into page context before E2E tests execute, preventing real browser extension wallet dependencies. Closes Fundable-Protocol#399 Closes Fundable-Protocol#464 Closes Fundable-Protocol#465
|
@utilityjnr Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
📝 WalkthroughWalkthroughThe changes enforce a five-minute minimum payment-stream duration, add ChangesPayment stream validation and testing
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/playwright.config.ts`:
- Around line 16-30: Replace the unsupported use.initScript configuration in the
Playwright setup with a reusable mock wallet script and a custom test fixture.
Extend the base test export with a context fixture that calls
context.addInitScript({ content: mockWalletScript }) before use(context),
ensuring wallet globals are installed before page scripts execute.
In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.test.tsx`:
- Around line 105-110: Extend the CreatePaymentStream form-layout test around
the Proceed button to fill valid fields, trigger the Proceed action, and assert
that the confirmation modal appears. Add a separate case with a duration under
five minutes, submit it, and assert the validation error produced by
validateEndTime. Keep the existing button-presence assertion only as supporting
coverage.
- Around line 41-43: Update the useDebouncedCallback mock in the Vitest setup to
remove the explicit any and use a generic function signature compatible with the
hook, preserving the behavior of returning the provided callback unchanged.
In `@apps/web/src/lib/stream-validation.ts`:
- Line 71: Update the start-time fallback in the stream validation logic to use
nullish coalescing so an explicit epoch value of 0 is preserved. Apply the same
change in calculateEndTime, retaining the current fallback only for null or
undefined values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7e1f2d0a-ab67-4e02-b95a-49d898cac635
📒 Files selected for processing (3)
apps/web/playwright.config.tsapps/web/src/components/modules/payment-stream/CreatePaymentStream.test.tsxapps/web/src/lib/stream-validation.ts
| initScript: ` | ||
| window.stellarWallet = { | ||
| address: 'GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF', | ||
| isConnected: true, | ||
| }; | ||
| window.freighterApi = { | ||
| isConnected: async () => true, | ||
| getPublicKey: async () => 'GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF', | ||
| signTransaction: async () => 'mock_signed_tx', | ||
| }; | ||
| window.albedo = { | ||
| publicKey: async () => ({ pubkey: 'GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF' }), | ||
| tx: async () => ({ signed_envelope_xdr: 'mock_signed_tx' }), | ||
| }; | ||
| `, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For @playwright/test1.48.0, isuse.initScripta supported Playwright Test configuration option? What fixture pattern should installbrowserContext.addInitScript for every E2E test context?
💡 Result:
In @playwright/test 1.48.0, use.initScript is not a supported configuration option in the standard Playwright Test configuration (playwright.config.ts) [1][2]. While some external tools, such as the Playwright MCP (Model Context Protocol) server, may support an initScript configuration option, it is not part of the core Playwright Test framework [3][4][5]. The recommended pattern to install browserContext.addInitScript for every E2E test context is to extend the built-in context fixture using Playwright's fixture system [1][6]. By creating a custom test fixture, you can ensure that the initialization script is automatically applied to every test context in a clean, reusable way [7][6]. To implement this, extend the base test object as follows: import { test as base, expect } from '@playwright/test'; export const test = base.extend({ // Override the context fixture context: async ({ context }, use) => { // Install the init script await context.addInitScript( => { // Your initialization code here window.localStorage.setItem('my-key', 'my-value'); }); // Continue with the test await use(context); }, }); This approach ensures that every page created within the test context will have the init script evaluated before any of the page's own scripts run [8][6]. Using fixtures is preferred over test.beforeEach hooks because it centralizes the configuration and ensures the script is applied consistently across all tests that use the extended test object [7][9].
Citations:
- 1: [Feature]
addInitScriptcontext option inplaywright.config.tsmicrosoft/playwright#11585 - 2: https://playwright.dev/docs/api/class-test
- 3: https://playwright.dev/mcp/configuration/options
- 4: https://github.com/microsoft/playwright-mcp/blob/main/config.d.ts
- 5: https://playwright.dev/agent-cli/configuration
- 6: https://www.developerload.com/playwright-before-each-for-all-spec-files
- 7: https://playwright.dev/docs/test-fixtures
- 8: https://playwright.dev/docs/api/class-browsercontext
- 9: https://github.com/microsoft/playwright-test/blob/master/docs/setup-teardown.md
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate config and related files =="
git ls-files | rg '(^|/)playwright\.config\.(ts|js|mjs|cjs)$|playwright' | sed -n '1,120p'
echo
echo "== apps/web/playwright.config.ts =="
if [ -f apps/web/playwright.config.ts ]; then
cat -n apps/web/playwright.config.ts
fi
echo
echo "== package versions =="
if [ -f package.json ]; then jq '.dependencies["`@playwright/test`"],.devDependencies["`@playwright/test`"]' package.json; fi
if [ -f apps/web/package.json ]; then jq '.dependencies["`@playwright/test`"],.devDependencies["`@playwright/test`"]' apps/web/package.json; fi
echo
echo "== usages of wallet globals in app =="
rg -n "stellarWallet|freighterApi|albedo|mock_signed_tx|GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF" .Repository: Fundable-Protocol/stellar_client_os
Length of output: 9574
Install the wallet mock through a Playwright fixture.
use.initScript is not a Playwright Test configuration option, so this wallet global is not added before page scripts:
const mockWalletScript = `
window.stellarWallet = {
address: 'GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF',
isConnected: true,
};
window.freighterApi = { isConnected: async () => true, getPublicKey: async () => 'GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF', signTransaction: async () => 'mock_signed_tx' };
window.albedo = { publicKey: async () => ({ pubkey: 'GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAWHF' }), tx: async () => ({ signed_envelope_xdr: 'mock_signed_tx' }) };
`;
export const test = base.extend({
context: async ({ context }, use) => {
await context.addInitScript({ content: mockWalletScript });
await use(context);
},
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/playwright.config.ts` around lines 16 - 30, Replace the unsupported
use.initScript configuration in the Playwright setup with a reusable mock wallet
script and a custom test fixture. Extend the base test export with a context
fixture that calls context.addInitScript({ content: mockWalletScript }) before
use(context), ensuring wallet globals are installed before page scripts execute.
| vi.mock("@/hooks/use-debounce-callback", () => ({ | ||
| useDebouncedCallback: (fn: any) => fn, | ||
| })); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
rg -n -C 3 --glob '*.ts' --glob '*.tsx' '\buseDebouncedCallback\b' apps/web/srcRepository: Fundable-Protocol/stellar_client_os
Length of output: 6906
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== test excerpt =="
sed -n '36,48p' apps/web/src/components/modules/payment-stream/CreatePaymentStream.test.tsx | cat -n
echo
echo "== hook =="
sed -n '1,40p' apps/web/src/hooks/use-debounce-callback.ts | cat -n
echo
echo "== ESLint config no-explicit-any =="
rg -n "no-explicit-any|`@typescript-eslint/parser`|typescript-eslint|parserOptions" . --glob '!node_modules' --glob '!dist' --glob '!build' | head -200Repository: Fundable-Protocol/stellar_client_os
Length of output: 11623
Remove the explicit any from the Vitest mock.
apps/web/eslint.config.mjs enables @typescript-eslint/no-explicit-any, so this mock declaration fails lint. Use a generic signature aligned with the hook:
Proposed fix
vi.mock("`@/hooks/use-debounce-callback`", () => ({
- useDebouncedCallback: (fn: any) => fn,
+ useDebouncedCallback: <T extends (...args: never[]) => unknown>(fn: T): T => fn,
}));📝 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.
| vi.mock("@/hooks/use-debounce-callback", () => ({ | |
| useDebouncedCallback: (fn: any) => fn, | |
| })); | |
| vi.mock("`@/hooks/use-debounce-callback`", () => ({ | |
| useDebouncedCallback: <T extends (...args: never[]) => unknown>(fn: T): T => fn, | |
| })); |
🧰 Tools
🪛 ESLint
[error] 42-42: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🪛 GitHub Check: test
[failure] 42-42:
Unexpected any. Specify a different type
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.test.tsx`
around lines 41 - 43, Update the useDebouncedCallback mock in the Vitest setup
to remove the explicit any and use a generic function signature compatible with
the hook, preserving the behavior of returning the provided callback unchanged.
Source: Linters/SAST tools
| it("shows Proceed button in form layout", () => { | ||
| render(<CreatePaymentStream />); | ||
|
|
||
| const proceedButton = screen.getByRole("button", { name: /proceed/i }); | ||
| expect(proceedButton).toBeTruthy(); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exercise the Proceed action, not just its presence.
This does not verify its handler. Submit valid fields and assert the confirmation modal; add a sub-five-minute duration case asserting the validation error. The handler is where this wizard applies validateEndTime.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/components/modules/payment-stream/CreatePaymentStream.test.tsx`
around lines 105 - 110, Extend the CreatePaymentStream form-layout test around
the Proceed button to fill valid fields, trigger the Proceed action, and assert
that the confirmation modal appears. Add a separate case with a duration under
five minutes, submit it, and assert the validation error produced by
validateEndTime. Keep the existing button-presence assertion only as supporting
coverage.
|
dont forget to offramp using https://stellar.fundable.finance/offramp its fast, free and p2p rates |
1 similar comment
|
dont forget to offramp using https://stellar.fundable.finance/offramp its fast, free and p2p rates |
2 participants
…s, inject mock wallet in Playwright
Detailed Resolution for Issues #399, #464, and #465:
Issue web(stream-validation): require minimum 5-minute duration for payment streams #399: web(stream-validation): require minimum 5-minute duration for payment streams
Issue web(CreatePaymentStream.test): add component render tests for stream creation wizard #464: web(CreatePaymentStream.test): add component render tests for stream creation wizard
Issue web(playwright.config): inject mock wallet provider in Playwright fixture #465: web(playwright.config): inject mock wallet provider in Playwright fixture
Closes #399
Closes #464
Closes #465
Summary by CodeRabbit
Bug Fixes
Tests