Add pledge receipt endpoint pdf - #723
Conversation
|
@sarah-obasi-analytics is attempting to deploy a commit to the ritik4ever's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@sarah-obasi-analytics 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 PR adds a PDF pledge receipt endpoint with integration tests, tightens campaign validation, changes Redis error handling, and replaces the CI workflow with consolidated application, coverage, and contract-test jobs. ChangesPledge receipt endpoint
Backend validation and resilience
CI and coverage workflow
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Client
participant ExpressReceiptRoute
participant campaignStore
participant PDFDocument
Client->>ExpressReceiptRoute: Request pledge receipt
ExpressReceiptRoute->>campaignStore: Fetch campaign and pledge
campaignStore-->>ExpressReceiptRoute: Return records
ExpressReceiptRoute->>PDFDocument: Generate receipt content
PDFDocument-->>Client: Stream PDF attachment
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.5)backend/src/index.tsFile contains syntax errors that prevent linting: Line 531: backend/src/services/campaignStore.tsFile contains syntax errors that prevent linting: Line 217: Illegal use of an export declaration not at the top level; Line 234: Illegal use of an export declaration not at the top level; Line 263: Illegal use of an export declaration not at the top level; Line 301: Illegal use of an export declaration not at the top level; Line 302: Illegal use of an export declaration not at the top level; Line 304: Illegal use of an export declaration not at the top level; Line 318: Illegal use of an export declaration not at the top level; Line 324: Illegal use of an export declaration not at the top level; Line 329: Illegal use of an export declaration not at the top level; Line 334: Illegal use of an export declaration not at the top level; Line 341: Illegal use of an export declaration not at the top level; Line 352: Illegal use of an export declaration not at the top level; Line 368: Illegal use of an export declaration not at the top level; Line 527: Illegal use of an export declaration not at ... [truncated 797 characters] ... use of an export declaration not at the top level; Line 1238: Illegal use of an export declaration not at the top level; Line 1272: Illegal use of an export declaration not at the top level; Line 1361: Illegal use of an export declaration not at the top level; Line 1383: Illegal use of an export declaration not at the top level; Line 1423: Illegal use of an export declaration not at the top level; Line 1436: Illegal use of an export declaration not at the top level; Line 1452: Illegal use of an export declaration not at the top level; Line 1497: Illegal use of an export declaration not at the top level; Line 1528: Illegal use of an export declaration not at the top level; Line 1538: Illegal use of an export declaration not at the top level; Line 1550: expected backend/src/validation/schemas.tsFile contains syntax errors that prevent linting: Line 120: Expected a property, a shorthand property, a getter, a setter, or a method but instead found '.'.; Line 126: expected 🔧 ESLint
backend/src/index.tsParsing error: ',' expected. backend/src/services/campaignStore.tsParsing error: '}' expected. backend/src/validation/schemas.tsParsing error: Identifier expected. 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: 8
🧹 Nitpick comments (1)
backend/src/services/cache.ts (1)
39-42: 🩺 Stability & Availability | 🔵 TrivialPreserve observability when Redis initialization fails.
This catch now prevents
initRedisCache().catch(...)inbackend/src/index.tsfrom observing connection failures, while the error is discarded entirely. Add structured logging or a metric before continuing without Redis so production cache outages remain diagnosable.🤖 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 39 - 42, Update the Redis initialization catch block in initRedisCache to record the caught error through the existing structured logger or metrics mechanism before resetting redisClient and isConnected, ensuring failures remain observable while preserving the current no-Redis fallback behavior.
🤖 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 @.github/workflows/ci.yml:
- Line 1: Normalize the CI workflow file formatting by converting all line
endings to LF and removing the extra trailing blank line at the end of the file,
while preserving the existing workflow content and structure.
- Around line 28-45: Update the CI steps for TypeScript Check, ESLint, Backend
Jest, Frontend Vitest, and Playwright E2E to remove the || echo fallbacks and
any --if-present or passWithNoTests options that allow failures or missing
scripts to pass. Invoke the required checks directly so every non-zero exit
fails the workflow.
- Around line 3-15: Update the workflow-level configuration in ci.yml to set
permissions to contents: read, and modify both checkout steps in the validate-pr
and corresponding other job to set persist-credentials: false. Preserve the
existing checkout actions and job behavior while restricting token access and
preventing credential persistence.
- Around line 22-23: Update the dependency-installation steps in the CI workflow
so the root npm install is followed by npm run install:all before backend tests
execute. Keep the existing backend test command unchanged, ensuring
child-package dependencies and the locally resolved backend Vitest installation
are available.
In `@backend/src/index.ts`:
- Around line 493-496: Update pledgeId validation in the request handler to
validate the entire req.params.pledgeId segment rather than relying on parseInt,
rejecting values with trailing characters or decimals. Require the resulting ID
to be a safe integer before querying, and preserve the existing AppError
response for invalid input.
- Around line 519-521: Update the receipt-generation flow around the
transactionHash check to reject pledges without a confirmed 64-character
hexadecimal transaction hash, including undefined or pending values, before
generating the PDF. Return an explicit pending-transaction response/error and
only render the receipt when the hash passes validation.
In `@backend/src/receiptEndpoint.test.ts`:
- Around line 69-73: Extend the receipt endpoint test around the existing
response assertions to parse res.body as a PDF and verify it is renderable.
Assert the extracted PDF text contains the campaign title, contributor, amount,
asset, timestamp, and transaction hash, while retaining the existing status,
header, Buffer, and non-empty checks.
In `@backend/src/services/cache.ts`:
- Around line 135-136: Update the catch block handling redisClient.quit() in the
cache service to reset isConnected to false and clear redisClient when closing
fails, ensuring isCacheAvailable() cannot report a failed client as available.
---
Nitpick comments:
In `@backend/src/services/cache.ts`:
- Around line 39-42: Update the Redis initialization catch block in
initRedisCache to record the caught error through the existing structured logger
or metrics mechanism before resetting redisClient and isConnected, ensuring
failures remain observable while preserving the current no-Redis fallback
behavior.
🪄 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: dd766467-8eb7-4dc7-b912-e839c368df2d
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.github/workflows/ci.ymlREADME.mdbackend/package.jsonbackend/src/index.tsbackend/src/receiptEndpoint.test.tsbackend/src/services/cache.tsbackend/src/services/campaignStore.tsbackend/src/validation/schemas.ts
|
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! |
1 similar comment
|
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! |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
backend/src/services/campaignStore.ts (4)
1029-1045: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSelect the average funding rate.
The query does not define
avg_funding_rate_pct.avgFundingRatePcttherefore always returns0. Add the aggregate expression to thisSELECT, with a zero-target guard.-->
🤖 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/campaignStore.ts` around lines 1029 - 1045, The campaign summary query in the aggregate flow around the visible SELECT must define avg_funding_rate_pct. Add an aggregate average funding-rate expression based on pledged_amount versus target_amount, guarding zero targets to avoid division errors, so the existing avgFundingRatePct mapping receives the computed value instead of defaulting to 0.
202-209: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRemove the duplicate
getContributorPledgedTotaldeclaration before the export.
backend/src/services/campaignStore.ts:211-212declaresfunction getContributorPledgedTotal(...)and then immediately repeats it asexport function getContributorPledgedTotal(...)inside the first body. This makes theexportnon-top-level and breaks TypeScript parsing. Keep one declaration/export boundary.🤖 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/campaignStore.ts` around lines 202 - 209, Remove the duplicate nested getContributorPledgedTotal declaration near getPledgeById, leaving a single top-level export function declaration so the function remains valid and exported.Source: Linters/SAST tools
1432-1435: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPass milliseconds to
calculateProgress.
nowis in Unix seconds for the SQL query.calculateProgresscompares itsatargument withcampaign.deadline * 1000. This call marks expired campaigns as open and returns an incorrecthoursLeft. Passnow * 1000.🤖 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/campaignStore.ts` around lines 1432 - 1435, Update the calculateProgress call in the campaign result construction to pass now converted from Unix seconds to milliseconds (now * 1000), matching its comparison with campaign.deadline * 1000 and preserving correct expired status and hoursLeft calculations.
491-504: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMake
campaign_faileddispatch single-shot.Both paths check
failed_atbefore an unconditional update. Concurrent list and detail requests can both observeNULLand dispatch duplicate webhooks. UseUPDATE ... WHERE id = ? AND failed_at IS NULL, then dispatch only whenchanges === 1.
backend/src/services/campaignStore.ts#L491-L504: condition the update and webhook on the update result.backend/src/services/campaignStore.ts#L530-L543: condition the update and webhook on the update result.🤖 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/campaignStore.ts` around lines 491 - 504, Make campaign_failed webhook dispatch single-shot in both campaignStore.ts sites at lines 491-504 and 530-543: change each failed_at update to include failed_at IS NULL in its WHERE clause, capture the update result, and dispatch the webhook only when changes equals 1; retain the existing eligibility checks and payload.README.md (1)
443-457: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the duplicated seed-database block and fix the malformed heading.
This section repeats the instructions already provided at Lines 427-441. The
Build:eedtext is also malformed. Keep one seed block and retain theBuild:heading before the build command.🤖 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 `@README.md` around lines 443 - 457, Remove the duplicated seed-database instructions from the README, keeping only one seed block. Correct the malformed “Build:eed” heading to “Build:” and preserve it before the build command.backend/src/services/cache.ts (1)
33-43: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winReplace the consecutive
catchclauses with one handler pertryblock.
catchcannot follow anothercatch. Lines 36, 40, 43, and 137 cause parser errors. Sincebackend/src/index.tscallsinitRedisCache()at startup, this prevents the backend from starting.
backend/src/services/cache.ts#L33-L43: MovelogInfo('redis_connected', ...)after a successfulawait redisClient.connect(). Keep one failurecatchthat logs the error and clearsredisClientandisConnected.backend/src/services/cache.ts#L135-L139: Keep onecatchforquit()failures. ClearredisClientand setisConnected = falsein that handler.Proposed fix
await redisClient.connect(); isConnected = true; - } catch (error) { - logInfo('redis_connected', {}, config.logLevel); } catch (error) { logError(error instanceof Error ? error : new Error(String(error)), { event: 'redis_connection_failed' }, config.logLevel); redisClient = null; isConnected = false; - } catch { - redisClient = null; - isConnected = false; - } catch { - redisClient = null; - isConnected = false; } @@ - } catch (error) { - // Ignore errors on close - } catch { + } catch { + redisClient = null; isConnected = false; }🤖 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 33 - 43, Replace the consecutive catch clauses in backend/src/services/cache.ts lines 33-43 with one try/catch flow: in the successful redisClient.connect() path, log redis_connected afterward; in the single failure handler, log the error and clear redisClient and isConnected. At lines 135-139, retain one catch for quit() failures and clear redisClient while setting isConnected to false.Source: Linters/SAST tools
backend/src/validation/schemas.ts (1)
120-134: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winReplace the existing schema properties instead of appending this chain.
At
backend/src/validation/schemas.ts:120,.min(4, ...)follows the completedacceptedTokensschema, not a property name. This is invalid TypeScript and creates duplicatetitle,description, andacceptedTokenskeys. Replace the definitions at lines 108-119 with the intended validated schema instead of keeping the old definitions plus a second chain.🤖 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` around lines 120 - 134, Replace the existing title, description, and acceptedTokens property definitions in the schema with the validated chain shown in the diff; do not append it after the completed acceptedTokens property. Ensure each property appears exactly once and the chain remains attached to its corresponding zod schema.Source: Linters/SAST tools
🤖 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.
Outside diff comments:
In `@backend/src/services/cache.ts`:
- Around line 33-43: Replace the consecutive catch clauses in
backend/src/services/cache.ts lines 33-43 with one try/catch flow: in the
successful redisClient.connect() path, log redis_connected afterward; in the
single failure handler, log the error and clear redisClient and isConnected. At
lines 135-139, retain one catch for quit() failures and clear redisClient while
setting isConnected to false.
In `@backend/src/services/campaignStore.ts`:
- Around line 1029-1045: The campaign summary query in the aggregate flow around
the visible SELECT must define avg_funding_rate_pct. Add an aggregate average
funding-rate expression based on pledged_amount versus target_amount, guarding
zero targets to avoid division errors, so the existing avgFundingRatePct mapping
receives the computed value instead of defaulting to 0.
- Around line 202-209: Remove the duplicate nested getContributorPledgedTotal
declaration near getPledgeById, leaving a single top-level export function
declaration so the function remains valid and exported.
- Around line 1432-1435: Update the calculateProgress call in the campaign
result construction to pass now converted from Unix seconds to milliseconds (now
* 1000), matching its comparison with campaign.deadline * 1000 and preserving
correct expired status and hoursLeft calculations.
- Around line 491-504: Make campaign_failed webhook dispatch single-shot in both
campaignStore.ts sites at lines 491-504 and 530-543: change each failed_at
update to include failed_at IS NULL in its WHERE clause, capture the update
result, and dispatch the webhook only when changes equals 1; retain the existing
eligibility checks and payload.
In `@backend/src/validation/schemas.ts`:
- Around line 120-134: Replace the existing title, description, and
acceptedTokens property definitions in the schema with the validated chain shown
in the diff; do not append it after the completed acceptedTokens property.
Ensure each property appears exactly once and the chain remains attached to its
corresponding zod schema.
In `@README.md`:
- Around line 443-457: Remove the duplicated seed-database instructions from the
README, keeping only one seed block. Correct the malformed “Build:eed” heading
to “Build:” and preserve it before the build command.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f31ae892-55d3-4160-9c59-ffcd1fb2795d
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
README.mdbackend/package.jsonbackend/src/index.tsbackend/src/services/cache.tsbackend/src/services/campaignStore.tsbackend/src/validation/schemas.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/package.json
- backend/src/index.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (8)
backend/src/validation/schemas.ts (3)
1-3: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRemove the duplicate import bindings.
z,extendZodWithOpenApi, andconfigare imported twice, which makesbackend/src/validation/schemas.tsfail TypeScript compilation. Keep one declaration for each binding.🤖 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` around lines 1 - 3, Remove the duplicate import declarations in schemas.ts for z, extendZodWithOpenApi, and config, preserving one import binding for each symbol and leaving their existing module sources unchanged.
111-119: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the duplicate description minimum.
createCampaignPayloadSchema.descriptionis parsed before the later fields run, so the.min(10, ...)branch exits when the 10–19-character input fails and the 20-character rule is never tested for that path. Remove one minimum and add a boundary test for the retained minimum.🤖 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` around lines 111 - 119, Update createCampaignPayloadSchema.description to retain only one minimum-length validation, removing the duplicate .min(10, ...) rule while preserving the intended minimum requirement. Add a boundary test covering the retained minimum length and verify inputs below it are rejected.
111-119: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate title length after sanitization.
sanitizeInput()expands<,>, and/into multi-character escapes. A title such asx<passes.max(100)once, but becomesx<after transformation; if the enforced limit applies to the stored/returned sanitized value, add a.refine((val) => val.length <= 100)after the title transform and cover boundary inputs.🤖 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` around lines 111 - 119, Update the title validation schema to enforce the 100-character limit after sanitizeInput transforms the value, adding a post-transform length refinement while preserving the existing minimum and maximum checks; cover titles whose sanitized output is exactly 100 characters and exceeds 100.backend/src/index.ts (3)
696-701: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winAdd
returnafter the validation-failure response.
sendValidationError(parsedId.issues)sends a response whenparsedId.okis false, but there is noreturnafterward. Execution falls through to line 703-704 and callsaddPledge(parsedId.value, body)with a value from a failed parse. This risks a second write attempt after headers are already sent, and it processes a pledge with an invalid campaign ID.🐛 Suggested fix
const parsedId = parseCampaignId(req.params.id); if (!parsedId.ok) { - sendValidationError(parsedId.issues); + sendValidationError(parsedId.issues); + return; }🤖 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 696 - 701, Update the validation-failure branch in the async request handler around parseCampaignId so it returns immediately after sendValidationError(parsedId.issues). Prevent execution from reaching addPledge with parsedId.value when parsing fails, while preserving the existing success path.
99-106: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCheck for a merge-conflict artifact: two statements on one line.
Line 106 combines the import closing brace and
export const app = express();on a single line with no separating newline:} from './services/campaignCache';export const app = express();. This is syntactically valid, but it is an unusual pattern that commonly results from a mishandled merge conflict resolution. Given the PR has documented unresolved merge conflicts, verify this line was not corrupted during a rebase or merge.✏️ Suggested formatting fix
-} from './services/campaignCache';export const app = express(); +} from './services/campaignCache'; + +export const app = express();🤖 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 99 - 106, Separate the campaignCache import declaration from the app export by placing `export const app = express();` on its own line after the import statement, and verify no merge-conflict content was lost around the import boundary.
530-554: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winResolve the syntax errors in
backend/src/index.tsbefore review.
- The cache handlers are embedded as bare
try...catch/awaitbodies without enclosing function declarations, soawait getCampaignCacheEntry(...)andawait setCampaignCacheEntry(...)are not inside async functions. That also leaves the preceding statements invalid beforecatch.- The top-level import block is merged into
export const app = express();, which mangles the module import/export boundary.The campaignStore syntax appears valid in the exported handler, so this comment applies only to
backend/src/index.ts.🤖 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 530 - 554, The campaign detail cache flow in backend/src/index.ts:530-554 must be enclosed in a valid async route-handler function so its try/catch structure and await calls are syntactically valid; preserve the existing campaign lookup and cache response behavior. Separate the top-level imports from export const app = express() in backend/src/index.ts:99-106 to restore the module boundary. No direct change is required in backend/src/services/campaignStore.ts:610-642 because its exported handler syntax is already valid.Source: Linters/SAST tools
backend/src/services/campaignStore.ts (2)
1390-1393: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse
toServiceErrorfor the not-found case inupdateCampaign.Every other not-found path in this file (
getCampaigncallers viatoServiceError('Campaign not found.', 404, 'NOT_FOUND')inaddPledge,reconcileOnChainPledge,refundContributor) throws throughtoServiceError, which carries astatusCode. Here, the error is built withObject.assign(new Error(...), { code: 'CAMPAIGN_NOT_FOUND' }), which has nostatusCodeproperty and a differently-named code. If the central error handler readserror.statusCode(as theAppError-based flow elsewhere implies), this path likely returns 500 instead of 404 to the client.🐛 Suggested fix
if (!campaign) { - throw Object.assign(new Error(`Campaign ${campaignId} not found`), { - code: 'CAMPAIGN_NOT_FOUND', - }); + throw toServiceError(`Campaign ${campaignId} not found`, 404, 'NOT_FOUND'); }🤖 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/campaignStore.ts` around lines 1390 - 1393, Update the not-found branch in updateCampaign to throw the established toServiceError result with the campaign-not-found message, HTTP status 404, and the repository’s standard NOT_FOUND code. Remove the Object.assign Error construction and preserve the existing successful update flow.
378-401: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAvoid sending raw FTS5 booleans into
MATCH.
ftsMatchTermis the value from user input after only space normalization, so uppercaseAND,OR, orNOTtokens are parsed as FTS5 operators. Use a literal quoted phrase, such as"${cleanQuery.replace(/\*/g, '')}"*, or lowercase before comparison.🤖 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/campaignStore.ts` around lines 378 - 401, Update the search-query handling around ftsMatchTerm so user-provided boolean words cannot be interpreted as FTS5 operators by MATCH. Build the FTS term as a quoted literal phrase, removing any wildcard characters before quoting while preserving the intended trailing prefix wildcard; keep the existing creator and campaign ID matching branches unchanged.
🧹 Nitpick comments (2)
backend/src/index.ts (1)
105-106: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd Helmet middleware to the Express app.
Static analysis flags
express()as missing Helmet security headers. Addhelmet()as early middleware to set standard protections (X-Content-Type-Options,X-Frame-Options,Strict-Transport-Security, etc.).🛡️ Suggested fix
+import helmet from 'helmet'; + export const app = express(); +app.use(helmet());🤖 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 105 - 106, Update the Express app initialization at app to import Helmet and register helmet() as early middleware immediately after express() is created, before application routes or other request handling, so standard security headers are applied.Source: Linters/SAST tools
backend/src/services/campaignStore.ts (1)
769-780: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated token-acceptance check into a shared helper.
addPledgeandreconcileOnChainPledgecontain the same token-matching block almost verbatim: direct canonical match, fallback toassetCodefor legacy entries, and asset-code-prefix matching. Extract this into a shared function, e.g.isTokenAcceptedByCampaign(campaign, tokenId, assetCode), to keep the matching rule consistent and avoid it drifting between the two call sites.♻️ Suggested refactor
+function isTokenAcceptedByCampaign( + campaign: CampaignRecord, + tokenId: string, + assetCode: string, +): boolean { + return campaign.acceptedTokens.some((accepted) => { + if (accepted === tokenId) return true; + if (!accepted.includes(':')) { + return accepted === assetCode; + } + return accepted.split(':')[0] === assetCode; + }); +}Then replace both inline blocks with
isTokenAcceptedByCampaign(campaign, tokenId, assetCode).Also applies to: 918-933
🤖 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/campaignStore.ts` around lines 769 - 780, Extract the duplicated token-matching logic from addPledge and reconcileOnChainPledge into a shared isTokenAcceptedByCampaign(campaign, tokenId, assetCode) helper. Preserve the existing direct canonical match, legacy asset-code fallback, and asset-code-prefix behavior, then replace both inline checks with calls to the helper.
🤖 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.
Outside diff comments:
In `@backend/src/index.ts`:
- Around line 696-701: Update the validation-failure branch in the async request
handler around parseCampaignId so it returns immediately after
sendValidationError(parsedId.issues). Prevent execution from reaching addPledge
with parsedId.value when parsing fails, while preserving the existing success
path.
- Around line 99-106: Separate the campaignCache import declaration from the app
export by placing `export const app = express();` on its own line after the
import statement, and verify no merge-conflict content was lost around the
import boundary.
- Around line 530-554: The campaign detail cache flow in
backend/src/index.ts:530-554 must be enclosed in a valid async route-handler
function so its try/catch structure and await calls are syntactically valid;
preserve the existing campaign lookup and cache response behavior. Separate the
top-level imports from export const app = express() in
backend/src/index.ts:99-106 to restore the module boundary. No direct change is
required in backend/src/services/campaignStore.ts:610-642 because its exported
handler syntax is already valid.
In `@backend/src/services/campaignStore.ts`:
- Around line 1390-1393: Update the not-found branch in updateCampaign to throw
the established toServiceError result with the campaign-not-found message, HTTP
status 404, and the repository’s standard NOT_FOUND code. Remove the
Object.assign Error construction and preserve the existing successful update
flow.
- Around line 378-401: Update the search-query handling around ftsMatchTerm so
user-provided boolean words cannot be interpreted as FTS5 operators by MATCH.
Build the FTS term as a quoted literal phrase, removing any wildcard characters
before quoting while preserving the intended trailing prefix wildcard; keep the
existing creator and campaign ID matching branches unchanged.
In `@backend/src/validation/schemas.ts`:
- Around line 1-3: Remove the duplicate import declarations in schemas.ts for z,
extendZodWithOpenApi, and config, preserving one import binding for each symbol
and leaving their existing module sources unchanged.
- Around line 111-119: Update createCampaignPayloadSchema.description to retain
only one minimum-length validation, removing the duplicate .min(10, ...) rule
while preserving the intended minimum requirement. Add a boundary test covering
the retained minimum length and verify inputs below it are rejected.
- Around line 111-119: Update the title validation schema to enforce the
100-character limit after sanitizeInput transforms the value, adding a
post-transform length refinement while preserving the existing minimum and
maximum checks; cover titles whose sanitized output is exactly 100 characters
and exceeds 100.
---
Nitpick comments:
In `@backend/src/index.ts`:
- Around line 105-106: Update the Express app initialization at app to import
Helmet and register helmet() as early middleware immediately after express() is
created, before application routes or other request handling, so standard
security headers are applied.
In `@backend/src/services/campaignStore.ts`:
- Around line 769-780: Extract the duplicated token-matching logic from
addPledge and reconcileOnChainPledge into a shared
isTokenAcceptedByCampaign(campaign, tokenId, assetCode) helper. Preserve the
existing direct canonical match, legacy asset-code fallback, and
asset-code-prefix behavior, then replace both inline checks with calls to the
helper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9d967429-1cd1-4670-90be-5418c9241239
📒 Files selected for processing (5)
README.mdbackend/package.jsonbackend/src/index.tsbackend/src/services/campaignStore.tsbackend/src/validation/schemas.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/package.json
|
@ritik4ever please merge PR, If there isnt any issue... |
2 participants
Pull Request
closes #563
What Changed
A clear and concise description of what this PR changes and why.
Related Issues
Testing Done
Describe the tests you ran and how to reproduce them.
Security Review
If this PR touches API endpoints, authentication, database queries, or contract code, check the applicable items from SECURITY_CHECKLIST.md. Paste relevant items below:
Checklist
npm test/cargo test)Screenshots (if applicable)
Add screenshots or recordings for visual changes.
Summary by CodeRabbit
New Features
Validation
Tests