Test/637 playwright pledge refund e2e - #688
Conversation
|
@Venrable18 is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe PR adds comprehensive API documentation, updates Soroban configuration and campaign query/reconciliation handling, expands validation and service diagnostics, and introduces Playwright coverage for pledge failure and refund lifecycle flows. ChangesBackend API and pledge lifecycle
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Playwright
participant DashboardPage
participant BackendAPI
Playwright->>BackendAPI: Create campaign with short deadline
Playwright->>DashboardPage: Submit partial pledge
DashboardPage->>BackendAPI: Create pledge
Playwright->>BackendAPI: Re-fetch campaign after deadline
BackendAPI-->>Playwright: Return failed campaign
Playwright->>DashboardPage: Trigger refund
DashboardPage->>BackendAPI: Process refund
BackendAPI-->>DashboardPage: Return refund confirmation
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 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 |
|
@Venrable18 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! 🚀 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (7)
playwright-report/index.html (1)
90-90: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid committing generated Playwright reports unless intentional.
This line embeds the full compressed report, screenshots, metadata, and test output into one opaque base64 payload. It will create huge diffs and may retain runtime test data in Git. Prefer CI artifact upload and add
playwright-report/to.gitignore; verify this artifact is intentionally versioned if that is required.🤖 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 `@playwright-report/index.html` at line 90, Remove the generated Playwright report artifact containing the playwrightReportBase64 payload from version control, add playwright-report/ to the repository ignore configuration, and retain report publication through CI artifact upload instead. Only keep the artifact tracked if the project explicitly requires intentional versioning.e2e/dashboard.ts (1)
36-38: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAvoid
waitUntil: 'networkidle'; it's explicitly discouraged by Playwright.Playwright's own docs mark
networkidleas discouraged for waiting/testing readiness — 'networkidle' - DISCOURAGED wait until there are no network connections for at least 500 ms. If the dashboard has any polling/long-lived requests, this can makegoto()hang toward its timeout on every test, slowing the whole suite and risking flakiness. Prefer waiting for a specific, meaningful locator (e.g. the campaigns table or a known heading) instead.♻️ Suggested fix
async goto() { - await this.page.goto('/', { waitUntil: 'networkidle' }); + await this.page.goto('/'); + await this.campaignsTable.waitFor({ state: 'visible' }); }🤖 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 `@e2e/dashboard.ts` around lines 36 - 38, Update the dashboard page object's goto() method to remove waitUntil: 'networkidle'. Navigate using the default load behavior, then explicitly wait for a meaningful dashboard-ready locator such as the campaigns table or known heading before returning.test-results/.last-run.json (1)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGenerated Playwright run artifact appears to be tracked in git.
test-results/.last-run.json(and apparentlyplaywright-report/index.html, also part of this change set) are generated on every test run. Committing them causes noisy diffs unrelated to code changes; consider addingtest-results/andplaywright-report/to.gitignoreinstead.🤖 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 `@test-results/.last-run.json` around lines 1 - 3, Remove the generated test-results/.last-run.json and playwright-report/index.html artifacts from version control, then update .gitignore to exclude the test-results/ and playwright-report/ directories so future Playwright runs do not track these generated files.e2e/pledge-refund-lifecycle-api.spec.ts (3)
61-63: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFixed 5s sleep to await deadline expiry is a minor flakiness risk.
A hard
setTimeoutpast a 4s deadline works but is an anti-pattern relative to Playwright's recommended web-first polling; considerexpect.poll(() => ...).toBe('failed')with a reasonable timeout instead, which will resolve as soon as the backend reports failure and fail fast with a clear message if it doesn't.🤖 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 `@e2e/pledge-refund-lifecycle-api.spec.ts` around lines 61 - 63, Replace the fixed 5-second delay in the “Wait for Deadline to Pass (Campaign Fails)” test.step with Playwright’s expect.poll, polling the campaign status until it reports “failed.” Configure a reasonable timeout and preserve the step’s purpose of waiting for backend failure while failing with a clear assertion if the deadline is not reached.
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated backend base URL into a constant.
http://localhost:3001is hardcoded four times in this spec. Extracting it to a local constant (or reading from an env var with this value as fallback) makes the port easy to change in one place and reduces copy/paste drift.Also applies to: 35-35, 50-50, 66-66
🤖 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 `@e2e/pledge-refund-lifecycle-api.spec.ts` at line 17, Extract the repeated http://localhost:3001 value used by the request calls in pledge-refund-lifecycle-api.spec.ts into a single local backend base URL constant, then reuse it for all four campaign API requests. Keep the current URL as the default value unless an existing environment configuration is already used.
8-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBoth new Playwright specs duplicate the same literal values with no shared source of truth: the backend base URL (
http://localhost:3001) and the test creator/contributor Stellar address. A small shared fixture/constants module would remove this duplication and make future changes (e.g. changing the backend port) a one-line edit.
e2e/pledge-refund-lifecycle-api.spec.ts#L8-L9: extractcreator/contributorinto a shared test constant instead of a duplicated literal.e2e/pledge-refund-lifecycle-api.spec.ts#L17-L66: extract the repeatedhttp://localhost:3001literal into a shared constant/helper (e.g.API_BASE_URL).e2e/pledge-refund-lifecycle.spec.ts#L28-L28: reuse the same shared creator address constant instead of redeclaring it here.e2e/pledge-refund-lifecycle.spec.ts#L37-L37: reuse the same sharedAPI_BASE_URLconstant instead of redeclaring the literal here.🤖 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 `@e2e/pledge-refund-lifecycle-api.spec.ts` around lines 8 - 9, Create a shared e2e fixture/constants module containing the creator/contributor Stellar address and API_BASE_URL, then update e2e/pledge-refund-lifecycle-api.spec.ts lines 8-9 and 17-66 to import and reuse them; update e2e/pledge-refund-lifecycle.spec.ts lines 28 and 37 likewise, removing the duplicated declarations and http://localhost:3001 literals.package.json (1)
41-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRun the full TypeScript project check from lint-staged.
String-form lint-staged tasks append staged files to the command, so
cd backend && tsc --noEmitruns astsc --noEmit file1.ts file2.ts. That makes TSC ignore the project config and references, checking only the provided files and skipping errors in files that consume the changed code. Use function-form entries that discard the file list and runtsc --noEmit --project tsconfig.jsonfor backend/frontend.Also applies to: 46-50
🤖 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 `@package.json` around lines 41 - 45, Update the lint-staged entries for backend and frontend TypeScript files to use function-form tasks that discard staged-file arguments and invoke the full project check with tsc --noEmit --project tsconfig.json. Preserve the existing ESLint and Prettier tasks, and apply the same change to both backend and frontend configurations.
🤖 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 `@API.md`:
- Around line 5-14: Update the API.md Base URL and Authentication sections to
reference ALLOWED_ORIGINS for production CORS and API_KEYS for authentication.
Document that production currently requires an Authorization: Bearer
<api-key> header, removing the public/future-only wording and ensuring
API_KEYS is configured.
- Around line 281-293: Update the deadline value in the request examples near
the shown payload, including the referenced examples, to use a dynamically
generated future timestamp or clearly future placeholder. Ensure every
executable example’s deadline is strictly later than the current time so
requests do not trigger INVALID_DEADLINE.
In `@backend/src/index.ts`:
- Around line 552-556: Update the reconciliation handler around addPledge to use
a dedicated reconciliation service or equivalent flow that persists the supplied
transaction hash and confirmedAt, deduplicates retries by transaction hash, and
allows valid on-chain pledges after the campaign deadline. Return the documented
{ campaign, transactionHash } response envelope instead of the current flattened
campaign payload, preserving the existing progress calculation.
In `@backend/src/services/cache.ts`:
- Around line 28-31: Update the Redis error listener in the cache service to
accept the emitted error and call logError(error, { event: "Redis client error"
}, LOG_LEVEL) before setting isConnected to false, preserving diagnostics for
asynchronous connection failures.
- Around line 137-139: Update the Redis shutdown logic around redisClient.quit()
so lifecycle cleanup runs in a finally block even when quitting throws. In that
cleanup, reset isConnected and clear redisClient, while preserving the existing
logError handling in the catch block.
In `@backend/src/validation/schemas.ts`:
- Line 5: Remove the unused CampaignStatus, CampaignSortField, and SortOrder
type imports from the campaignStore import in schemas.ts, unless they are
required elsewhere in the file; keep only imports that are actually referenced.
In `@e2e/pledge-refund-lifecycle.spec.ts`:
- Around line 81-88: Mark the “Refund Pledge” test step or enclosing test as
skipped/fixme, matching the documented rationale and treatment in
pledge-refund-lifecycle-api.spec.ts. Do not execute dashboard.refund() or assert
the success message until Soroban RPC verification is mocked or a real testnet
flow is available; preserve the remaining lifecycle coverage.
In `@package.json`:
- Around line 35-36: Add a package.json "prepare" script invoking Husky so
installation automatically sets up the existing .husky/pre-commit hook and its
lint-staged checks. Preserve the current scripts and dependencies.
---
Nitpick comments:
In `@e2e/dashboard.ts`:
- Around line 36-38: Update the dashboard page object's goto() method to remove
waitUntil: 'networkidle'. Navigate using the default load behavior, then
explicitly wait for a meaningful dashboard-ready locator such as the campaigns
table or known heading before returning.
In `@e2e/pledge-refund-lifecycle-api.spec.ts`:
- Around line 61-63: Replace the fixed 5-second delay in the “Wait for Deadline
to Pass (Campaign Fails)” test.step with Playwright’s expect.poll, polling the
campaign status until it reports “failed.” Configure a reasonable timeout and
preserve the step’s purpose of waiting for backend failure while failing with a
clear assertion if the deadline is not reached.
- Line 17: Extract the repeated http://localhost:3001 value used by the request
calls in pledge-refund-lifecycle-api.spec.ts into a single local backend base
URL constant, then reuse it for all four campaign API requests. Keep the current
URL as the default value unless an existing environment configuration is already
used.
- Around line 8-9: Create a shared e2e fixture/constants module containing the
creator/contributor Stellar address and API_BASE_URL, then update
e2e/pledge-refund-lifecycle-api.spec.ts lines 8-9 and 17-66 to import and reuse
them; update e2e/pledge-refund-lifecycle.spec.ts lines 28 and 37 likewise,
removing the duplicated declarations and http://localhost:3001 literals.
In `@package.json`:
- Around line 41-45: Update the lint-staged entries for backend and frontend
TypeScript files to use function-form tasks that discard staged-file arguments
and invoke the full project check with tsc --noEmit --project tsconfig.json.
Preserve the existing ESLint and Prettier tasks, and apply the same change to
both backend and frontend configurations.
In `@playwright-report/index.html`:
- Line 90: Remove the generated Playwright report artifact containing the
playwrightReportBase64 payload from version control, add playwright-report/ to
the repository ignore configuration, and retain report publication through CI
artifact upload instead. Only keep the artifact tracked if the project
explicitly requires intentional versioning.
In `@test-results/.last-run.json`:
- Around line 1-3: Remove the generated test-results/.last-run.json and
playwright-report/index.html artifacts from version control, then update
.gitignore to exclude the test-results/ and playwright-report/ directories so
future Playwright runs do not track these generated files.
🪄 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: 2816a6d4-3274-41b2-9ea0-dd1dc5c8be11
⛔ Files ignored due to path filters (2)
backend/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (20)
API.mdCONTRIBUTING.mdbackend/src/config.tsbackend/src/index.tsbackend/src/services/cache.tsbackend/src/services/eventHistory.tsbackend/src/services/eventIndexer.tsbackend/src/services/sorobanRpc.tsbackend/src/validateEnv.tsbackend/src/validation/schemas.tsbackend/src/validation/urlSafety.tse2e/dashboard.tse2e/pledge-refund-lifecycle-api.spec.tse2e/pledge-refund-lifecycle.spec.tsnpxpackage.jsonplaywright-report/index.htmlplaywright.config.tsstellar-goal-vault@1.0.0test-results/.last-run.json
| ## Base URL | ||
|
|
||
| - **Local Development**: `http://localhost:3000` | ||
| - **Production**: Configured via `CORS_ALLOWED_ORIGINS` | ||
|
|
||
| ## Authentication | ||
|
|
||
| **Current Status**: Public API (no authentication required) | ||
|
|
||
| **Future**: API key authentication will be required in production. Configure via `API_KEY` environment variable. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Document the actual production CORS and authentication contract.
Production already mounts API-key authentication, which reads API_KEYS; this section incorrectly says authentication is future-only and names API_KEY. It also names CORS_ALLOWED_ORIGINS, while the backend validates ALLOWED_ORIGINS. Following this documentation can leave API_KEYS empty, causing the middleware to accept any Bearer token.
Update the exact variable names and document the current production Authorization: Bearer <api-key> requirement.
🤖 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 `@API.md` around lines 5 - 14, Update the API.md Base URL and Authentication
sections to reference ALLOWED_ORIGINS for production CORS and API_KEYS for
authentication. Document that production currently requires an Authorization:
Bearer <api-key> header, removing the public/future-only wording and
ensuring API_KEYS is configured.
| { | ||
| "creator": "GAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA", | ||
| "title": "Clean Water Initiative", | ||
| "description": "Raising funds to provide clean water access.", | ||
| "acceptedTokens": ["USDC", "XLM"], | ||
| "targetAmount": 1000, | ||
| "deadline": 1705334400, | ||
| "metadata": { | ||
| "imageUrl": "https://example.com/image.jpg", | ||
| "externalLink": "https://example.com" | ||
| }, | ||
| "maxPerContributor": 500 | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Use future timestamps in executable examples.
1705334400 is January 15, 2024; the backend rejects deadlines at or before the current time. These examples therefore fail with INVALID_DEADLINE on July 29, 2026. Use a dynamically generated future timestamp or a clearly future placeholder in every request example.
Also applies to: 349-358, 1018-1028
🤖 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 `@API.md` around lines 281 - 293, Update the deadline value in the request
examples near the shown payload, including the referenced examples, to use a
dynamically generated future timestamp or clearly future placeholder. Ensure
every executable example’s deadline is strictly later than the current time so
requests do not trigger INVALID_DEADLINE.
| const body = req.body as z.infer<typeof reconcilePledgePayloadSchema>; | ||
| const campaign = addPledge(parsedId.value, body); | ||
| invalidateCampaignCache(); | ||
| res.status(result.existing ? 200 : 201).json({ | ||
| data: { | ||
|
|
||
| }, | ||
| res.status(201).json({ | ||
| data: { ...campaign, progress: calculateProgress(campaign) }, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Preserve transaction identity in the reconciliation path.
addPledge writes transaction_hash as NULL, ignores confirmedAt, has no transaction-hash idempotency, and rejects pledges after the current deadline. A valid on-chain pledge reconciled late can therefore be lost or rejected, while retries can create duplicate local pledges. The response also no longer matches the documented { campaign, transactionHash } envelope in API.md.
Restore the dedicated reconciliation service, or extend this path to persist and deduplicate by transaction hash before returning the documented response.
Proposed direction
- const campaign = addPledge(parsedId.value, body);
+ const result = reconcileOnChainPledge(parsedId.value, body);
- data: { ...campaign, progress: calculateProgress(campaign) },
+ data: {
+ campaign: {
+ ...result.campaign,
+ progress: calculateProgress(result.campaign),
+ },
+ transactionHash: result.transactionHash,
+ },📝 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.
| const body = req.body as z.infer<typeof reconcilePledgePayloadSchema>; | |
| const campaign = addPledge(parsedId.value, body); | |
| invalidateCampaignCache(); | |
| res.status(result.existing ? 200 : 201).json({ | |
| data: { | |
| }, | |
| res.status(201).json({ | |
| data: { ...campaign, progress: calculateProgress(campaign) }, | |
| const body = req.body as z.infer<typeof reconcilePledgePayloadSchema>; | |
| const result = reconcileOnChainPledge(parsedId.value, body); | |
| invalidateCampaignCache(); | |
| res.status(201).json({ | |
| data: { | |
| campaign: { | |
| ...result.campaign, | |
| progress: calculateProgress(result.campaign), | |
| }, | |
| transactionHash: result.transactionHash, | |
| }, |
🤖 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 `@backend/src/index.ts` around lines 552 - 556, Update the reconciliation
handler around addPledge to use a dedicated reconciliation service or equivalent
flow that persists the supplied transaction hash and confirmedAt, deduplicates
retries by transaction hash, and allows valid on-chain pledges after the
campaign deadline. Return the documented { campaign, transactionHash } response
envelope instead of the current flattened campaign payload, preserving the
existing progress calculation.
| redisClient.on("error", () => { | ||
|
|
||
| isConnected = false; | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the cache implementation and related Redis usage.
fd -a 'cache\.ts$' . || true
printf '\n--- cache.ts outline ---\n'
ast-grep outline backend/src/services/cache.ts --view expanded || true
printf '\n--- cache.ts contents ---\n'
cat -n backend/src/services/cache.ts
printf '\n--- Redis client usage searches ---\n'
rg -n 'redisClient|new IORedis|createClient|on\("error"|connect\(\)|throw|logger|console\.(error|log|warn)' backend/src -S || trueRepository: ritik4ever/stellar-goal-vault
Length of output: 18077
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- logger.ts ---'
cat -n backend/src/logger.ts
printf '\n%s\n' '--- cache init call sites ---'
rg -n 'initRedisCache\(|getCacheValue\(|setCacheValue\(|isCacheAvailable\(' backend/src -S || true
printf '\n%s\n' '--- cache tests ---'
fd -a 'cache.*\.test\.(ts|js)$' backend/src || true
if [ -f backend/src/services/cache.test.ts ]; then
cat -n backend/src/services/cache.test.ts
fiRepository: ritik4ever/stellar-goal-vault
Length of output: 5176
Preserve and log asynchronous Redis errors.
The empty error-listener signature discards the actual Redis error. Connection failures after startup bypass the connect() catch block, so isCacheAvailable() can switch away from the Redis client without any diagnostic record. Log the emitted error with logError(error, { event: "Redis client error" }, LOG_LEVEL) before updating state.
🤖 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 `@backend/src/services/cache.ts` around lines 28 - 31, Update the Redis error
listener in the cache service to accept the emitted error and call
logError(error, { event: "Redis client error" }, LOG_LEVEL) before setting
isConnected to false, preserving diagnostics for asynchronous connection
failures.
| } catch (error) { | ||
| logError(error, { event: "Failed to close Redis connection" }, LOG_LEVEL); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the relevant cache service around the reported lines and lifecycle state uses.
if [ -f backend/src/services/cache.ts ]; then
echo "== cache.ts outline =="
ast-grep outline backend/src/services/cache.ts || true
echo
echo "== cache.ts lines 1-220 =="
sed -n '1,220p' backend/src/services/cache.ts | nl -ba
else
echo "backend/src/services/cache.ts not found"
fd -i 'cache\.ts$|cache.*\.ts$' .
fiRepository: ritik4ever/stellar-goal-vault
Length of output: 940
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== cache.ts lines 1-220 =="
sed -n '1,220p' backend/src/services/cache.ts
echo
echo "== cache.ts lifecycle/getter/close/stat occurrences =="
rg -n "redisClient|isConnected|initRedisCache|closeRedisCache|isCacheAvailable|getCacheStats|throw|catch|finally|quit|connect|disconnect" backend/src/services/cache.tsRepository: ritik4ever/stellar-goal-vault
Length of output: 5386
Reset Redis state when closing fails.
If redisClient.quit() throws, isConnected remains true and the stale client remains assigned. Move the lifecycle cleanup into a finally block and clear redisClient so cache availability checks don’t report a failed shutdown as an active connection.
🤖 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 `@backend/src/services/cache.ts` around lines 137 - 139, Update the Redis
shutdown logic around redisClient.quit() so lifecycle cleanup runs in a finally
block even when quitting throws. In that cleanup, reset isConnected and clear
redisClient, while preserving the existing logError handling in the catch block.
| import { extendZodWithOpenApi } from '@asteasolutions/zod-to-openapi'; | ||
| import { config } from '../config'; | ||
| import { httpsOnlyUrlSchema } from './urlSafety'; | ||
| import type { CampaignStatus, CampaignSortField, SortOrder } from "../services/campaignStore"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Remove the unused type imports.
ESLint reports CampaignStatus, CampaignSortField, and SortOrder as unused on this line. Remove the import or use the types; otherwise the lint check remains failing.
Proposed fix
-import type { CampaignStatus, CampaignSortField, SortOrder } from "../services/campaignStore";📝 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.
| import type { CampaignStatus, CampaignSortField, SortOrder } from "../services/campaignStore"; |
🧰 Tools
🪛 ESLint
[error] 5-5: 'CampaignStatus' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 5-5: 'CampaignSortField' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 5-5: 'SortOrder' is defined but never used.
(@typescript-eslint/no-unused-vars)
🤖 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 `@backend/src/validation/schemas.ts` at line 5, Remove the unused
CampaignStatus, CampaignSortField, and SortOrder type imports from the
campaignStore import in schemas.ts, unless they are required elsewhere in the
file; keep only imports that are actually referenced.
Source: Linters/SAST tools
| await test.step('Refund Pledge', async () => { | ||
| // Verify refund button is visible/enabled for failed campaigns | ||
| await expect(dashboard.refundButton).toBeVisible(); | ||
|
|
||
| await dashboard.refund(); | ||
|
|
||
| await expect(page.locator('text=Refund processed successfully')).toBeVisible(); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
This step is a known failure and will break npm run test:e2e / CI.
Per the PR's own description, refund requires real Soroban RPC verification that isn't available in this environment, and the attached "Test Steps" screenshot shows exactly this step failing. Unlike the API spec (which explicitly skips the refund assertion with an explanatory comment), this UI spec still exercises the full refund flow and asserts success, so running the suite will always report a failure here.
Mark this step (or the whole test) as test.fixme()/test.skip() until Soroban RPC can be mocked or a real testnet flow is wired up, mirroring the rationale already documented in pledge-refund-lifecycle-api.spec.ts.
♻️ Suggested fix
- test('should complete full pledge+refund lifecycle (Create -> Pledge -> Verify Progress -> Fail -> Refund -> Verify Refunded)', async ({
+ test.fixme('should complete full pledge+refund lifecycle (Create -> Pledge -> Verify Progress -> Fail -> Refund -> Verify Refunded)', async ({
page,
request,
}) => {🤖 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 `@e2e/pledge-refund-lifecycle.spec.ts` around lines 81 - 88, Mark the “Refund
Pledge” test step or enclosing test as skipped/fixme, matching the documented
rationale and treatment in pledge-refund-lifecycle-api.spec.ts. Do not execute
dashboard.refund() or assert the success message until Soroban RPC verification
is mocked or a real testnet flow is available; preserve the remaining lifecycle
coverage.
| "husky": "^9.1.6", | ||
| "lint-staged": "^15.2.10", |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm husky is wired up via a prepare script and .husky/ hook files exist
jq '.scripts.prepare' package.json
fd -H '^\.husky$' -t d
fd . .husky 2>/dev/nullRepository: ritik4ever/stellar-goal-vault
Length of output: 198
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "package scripts:"
jq -r '.scripts // {} | to_entries[] | "\(.key)=\(.value)"' package.json
echo
echo ".husky files:"
git ls-files .husky || true
echo
echo ".husky/pre-commit:"
cat -n .husky/pre-commit 2>/dev/null || trueRepository: ritik4ever/stellar-goal-vault
Length of output: 902
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "package scripts:"
jq -r '.scripts // {} | to_entries[] | "\(.key)=\(.value)"' package.json
echo
echo ".husky files:"
git ls-files .husky || true
echo
echo ".husky/pre-commit:"
if [ -f .husky/pre-commit ]; then
cat -n .husky/pre-commit
fiRepository: ritik4ever/stellar-goal-vault
Length of output: 902
Add the missing Husky install hook.
.husky/pre-commit runs lint-staged, but the repo has no "prepare": "husky" script, so npm install won’t install the hook automatically. Add the prepare script so new contributors get the pre-commit checks, or document that hooks must be set up manually.
🤖 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 `@package.json` around lines 35 - 36, Add a package.json "prepare" script
invoking Husky so installation automatically sets up the existing
.husky/pre-commit hook and its lint-staged checks. Preserve the current scripts
and dependencies.
|
Hi @Venrable18, This PR could not be merged because it has merge conflicts with the target branch. Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged. Thank you! |
|
Hi @Venrable18, This PR could not be merged because it has merge conflicts with the target branch. Please resolve the merge conflicts, push the updated changes, and the PR can be reviewed and merged. Thank you! |
2 participants
Closes #637
Summary:
Task: Add Playwright E2E tests for full pledge and refund flow
What Was Accomplished:
✅ Created API-based E2E test (pledge-refund-lifecycle-api.spec.ts) covering: create campaign → pledge → verify progress → fail (past deadline)
✅ Fixed backend TypeScript compilation errors
✅ Rebuilt better-sqlite3 native module for Node compatibility
✅ Installed Playwright browsers
✅ Configured npm run test:e2e in package.json
✅ Test passes on Chromium: 1 passed (10.4s)
✅ Screenshot on failure configured in playwright.config.ts
✅ Added Soroban environment variables to Playwright config
However, the following loopholes held the test completion back:
❌ Refund E2E step - requires real Soroban RPC verification (backend makes actual network calls to Soroban nodes)
Summary by CodeRabbit