Conversation
WalkthroughThis pull request adds server-side validation of response data across multiple API endpoints. Validation checks are introduced in v1 and v2 client and management response handlers, occurring before response creation or updates. The validation utilities are consolidated from v2-specific paths to a unified location at 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/api/v1/client/[environmentId]/responses/route.ts (1)
121-144:⚠️ Potential issue | 🟠 MajorAdd "other option length" validation to V1 client POST route.
This endpoint validates file uploads and response data but is missing
validateOtherOptionLengthForMultipleChoice, which is present in the V2 client POST route (line 97) and V1 client PUT route (line 98). Without this validation, responses with "other" options exceeding the character limit can be created through this endpoint while being rejected by other endpoints.
🤖 Fix all issues with AI agents
In `@apps/web/app/api/v2/client/`[environmentId]/responses/route.ts:
- Around line 15-18: The import of formatValidationErrorsForV1Api and
validateResponseData in route.ts is using the wrong path; update the import to
point to the centralized validation utilities at "@/modules/api/lib/validation"
so that formatValidationErrorsForV1Api and validateResponseData are imported
from the correct module; keep the imported symbol names unchanged and verify any
other usages in route.ts refer to those same function names.
🧹 Nitpick comments (2)
apps/web/app/api/v1/management/responses/[responseId]/route.ts (1)
144-161: Consider: No data merging unlike V1 client PUT route.This V1 management PUT validates
responseUpdate.datadirectly, while the V1 client PUT route atapps/web/app/api/v1/client/[environmentId]/responses/[responseId]/route.ts(lines 118-131) merges existing response data with the update before validation:const mergedData = { ...response.data, ...inputValidation.data.data, };This difference may be intentional if management APIs expect complete data, but it creates inconsistent validation behavior between endpoints. When
finished: trueis sent with partial data, the management API will report "required" errors for missing fields, while the client API will validate the merged state.apps/web/modules/api/lib/validation.ts (1)
45-50: Avoid repeatedObject.keyswork during partial validation.
Object.keys(responseData)is recalculated for every element. Cache once to keep this O(n) and cleaner.♻️ Suggested refactor
- if (!finished) { - elementsToValidate = allElements.filter((element) => Object.keys(responseData).includes(element.id)); - } + if (!finished) { + const responseKeys = new Set(Object.keys(responseData)); + elementsToValidate = allElements.filter((element) => responseKeys.has(element.id)); + }
pandeymangg
left a comment
There was a problem hiding this comment.
Looks good! Just a small comment, pls take a look 🙏
|



Problem
Validation rules were not being applied when creating or updating responses through the client APIs (
/api/v1/clientand/api/v2/client). While the management APIs correctly validated response data against survey validation rules, the client API endpoints were missing this validation logic entirely.Additionally, client APIs receive partial responses as users progress through surveys (incremental updates), which required special handling to avoid false "required" errors for questions the user hasn't reached yet.
Solution
1. Enhanced
validateResponseDatafunctionUpdated the shared
validateResponseDatafunction to support partial response validation by adding afinishedparameter:finished: false(in-progress): Only validates fields that are present in the current payload, preventing "required" errors for unanswered questionsfinished: true(completed): Validates all fields to ensure the entire survey is complete and valid2. Centralized validation functions
Moved all validation-related functions to a shared location (
apps/web/modules/api/lib/validation.ts) since they're used by both v1 and v2 APIs (client and management):validateResponseData- Core validation logic (shared)formatValidationErrorsForV2Api- Formats errors for v2 management APIs (returnsApiErrorDetails[])formatValidationErrorsForV1Api- Formats errors for v1 APIs and v2 client APIs (returnsRecord<string, string>)3. Added validation to client API endpoints
Implemented validation in all client API response endpoints:
POST endpoints (create response):
apps/web/app/api/v1/client/[environmentId]/responses/route.tsapps/web/app/api/v2/client/[environmentId]/responses/route.tsPUT endpoints (update response):
apps/web/app/api/v1/client/[environmentId]/responses/[responseId]/route.tsapps/web/app/api/v2/client/[environmentId]/responses/[responseId]/route.ts(re-exports v1 handler)4. Updated management API endpoints
Updated management API endpoints to explicitly pass
finished: trueto maintain existing behavior:apps/web/modules/api/v2/management/responses/route.tsapps/web/modules/api/v2/management/responses/[responseId]/route.tsapps/web/app/api/v1/management/responses/route.tsapps/web/app/api/v1/management/responses/[responseId]/route.tsChanges
Files Modified
Created:
apps/web/modules/api/lib/validation.tsfinishedparameter support tovalidateResponseDataformatValidationErrorsForApi→formatValidationErrorsForV2Apifor clarityUpdated: All client API response routes (v1 and v2)
finishedflagUpdated: All management API response routes (v1 and v2)
finished: truefor full validationUpdated: Test file
apps/web/modules/api/lib/validation.test.tsfinished: false)Deleted:
apps/web/modules/api/v2/management/responses/lib/validation.tsFiles Deleted
apps/web/modules/api/v2/management/responses/lib/validation.ts(moved to shared location)apps/web/app/api/v2/client/[environmentId]/responses/lib/validation.ts(no longer needed, using shared function)Testing
All existing tests pass, including:
modules/api/lib/validation.test.tsImpact
Example
Before: A client API request with invalid email format would succeed without validation.
After: The same request now returns a proper validation error:
{ "code": "bad_request", "message": "Validation failed", "details": { "response.data.email": "Please enter a valid email address" } }Fixes https://github.com/formbricks/internal/issues/1306