feat: Option driven data payload with schema validation #7
Conversation
📝 WalkthroughWalkthroughRefactors AndroidInterface to emit validated "summary_data" payloads including explicit Changes
Sequence DiagramsequenceDiagram
participant Caller
participant AndroidInterface
participant ValidateV1Schema
participant Logger
Caller->>AndroidInterface: logSummaryData(data, options?)
AndroidInterface->>AndroidInterface: getBaseParams()
AndroidInterface->>AndroidInterface: createTimestamp()
AndroidInterface->>AndroidInterface: build payload (baseParams + data + options + timestamp + collection)
AndroidInterface->>ValidateV1Schema: validatePayload(payload)
ValidateV1Schema-->>AndroidInterface: validated payload
AndroidInterface->>Logger: emit log event
Logger-->>Caller: confirmation/ack
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)src/android-interface/android-interface.spec.ts (1)
🔇 Additional comments (4)
✏️ Tip: You can disable this entire section by setting 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: 2
🤖 Fix all issues with AI agents
In @package.json:
- Around line 49-50: package.json currently pins "zod": "^4.3.5" which is a
breaking major that requires migrating Zod v3 APIs; either pin to a Zod v3
release (e.g., change the dependency to "zod": "^3.21.4") to avoid runtime
breaks, or perform the migration: update all imports from "zod" usages, search
for and adapt ZodError handling and thrown/format shapes (references to
ZodError, .errors, .format), update parse APIs and return shapes (safeParse
usage), and adjust object schema strictness/object/strict() and transform APIs;
run the official Zod v3→v4 codemods and tests to validate changes before
merging.
In @src/android-interface/android-interface.ts:
- Around line 5-17: The interface AndroidInterfaceOptions declares app_id and
cr_user_id as required but runtime/constructor accepts partial options and uses
DEFAULT_OPTIONS; update the type to match runtime by making app_id and
cr_user_id optional (app_id? and cr_user_id?) on AndroidInterfaceOptions so
callers can omit them, and ensure DEFAULT_OPTIONS still satisfies
AndroidInterfaceOptions after this change; verify any code referencing
AndroidInterfaceOptions (e.g., the constructor that consumes options) handles
possibly undefined app_id/cr_user_id appropriately.
🧹 Nitpick comments (4)
src/android-interface/schema-validators.ts (1)
9-15: Schema missingoptionsfield present inAppEventPayloadtype.The
ValidateV1Schemadoes not include theoptionsfield, butAppEventPayloadintypes.d.tsdefinesoptions?: AppEventPayloadOptions. By default, Zod strips unknown keys during parsing, so the validated payload returned fromparse()will not includeoptions.However, since the validation result isn't used (the original
payloadobject is passed tologMessage), this works correctly. If you later use the parsed result, theoptionsfield would be lost.Consider adding the
optionsfield to the schema for completeness:♻️ Suggested schema update
+import { AppEventPayloadOptions } from './types'; + export const ValidateV1Schema = z.object({ cr_user_id: requiredString('CR User ID'), app_id: requiredString('App ID'), collection: requiredString('Collection'), data: z.record(z.string(), z.any()), - timestamp: requiredString('Event ISO-8601 timestamp') + timestamp: requiredString('Event ISO-8601 timestamp'), + options: z.record(z.string(), z.enum(['add', 'replace'])).optional() });src/android-interface/types.d.ts (1)
11-18: Consider usingRecord<string, any>for thedatafield.For consistency with the Zod schema (
z.record(z.string(), z.any())), consider typingdataasRecord<string, any>instead ofany. This provides slightly better type safety while maintaining flexibility.♻️ Suggested change
export interface AppEventPayload { cr_user_id: string; app_id: string; collection: AppEventPayloadCollection; - data: any; + data: Record<string, any>; options?: AppEventPayloadOptions; timestamp: string; }src/android-interface/android-interface.spec.ts (1)
86-99: Consider adding negative test cases for validation failures.The current tests cover successful scenarios. Consider adding tests that verify validation errors are handled gracefully (e.g., missing required fields, invalid data types). This would validate the
catchblock behavior inlogSummaryData.🧪 Example negative test
test('should warn and not call logMessage when validation fails', () => { const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); // Force validation failure by mocking validatePayload to throw // or by testing with invalid payload structure androidInterface.logSummaryData(null as any); expect(mockLogMessage).not.toHaveBeenCalled(); expect(consoleSpy).toHaveBeenCalled(); consoleSpy.mockRestore(); });src/android-interface/android-interface.ts (1)
42-42: Consider adding type safety for the global namespace access.The
window[this.options.namespace]access is not type-safe. Consider declaring an interface for the expected Android bridge methods.♻️ Suggested type declaration
// Add to types.d.ts or at the top of this file declare global { interface Window { Android?: { logMessage: (message: string) => void; }; [key: string]: any; } }Then add a runtime check:
+ const bridge = window[this.options.namespace!]; + if (!bridge?.logMessage) { + throw new Error(`Android bridge not found at window.${this.options.namespace}`); + } - window[this.options.namespace].logMessage(JSON.stringify(payload)); + bridge.logMessage(JSON.stringify(payload));
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.jsonsrc/android-interface/android-interface.spec.tssrc/android-interface/android-interface.tssrc/android-interface/schema-validators.tssrc/android-interface/types.d.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/android-interface/android-interface.spec.ts (1)
src/android-interface/android-interface.ts (2)
AndroidInterface(19-69)DEFAULT_OPTIONS(12-17)
src/android-interface/android-interface.ts (2)
src/android-interface/types.d.ts (3)
AppEventPayloadVersion(1-1)AppEventPayloadOptions(7-9)AppEventPayload(11-18)src/android-interface/schema-validators.ts (1)
ValidateV1Schema(9-15)
🔇 Additional comments (5)
src/android-interface/schema-validators.ts (1)
3-7: LGTM — Zod v4 error customization syntax is correct.The
errorcallback approach aligns with Zod 4's unified error customization API. The conditional logic for distinguishing missing vs. wrong-type inputs provides clear error messages.src/android-interface/types.d.ts (1)
1-9: LGTM — Well-structured type definitions.The literal types (
'v1','summary_data','add' | 'replace') provide good type safety, and the interface design cleanly separates concerns.src/android-interface/android-interface.spec.ts (1)
56-84: LGTM — Good test coverage for the happy path.The tests effectively validate payload structure, timestamp format, and the integration with the Android bridge mock.
src/android-interface/android-interface.ts (2)
29-46: LGTM — Clean payload construction and validation flow.The
logSummaryDatamethod properly builds the payload, validates it, and handles errors gracefully with a warning log. The use oftry/catchensures validation failures don't crash the application.
48-68: LGTM — Well-organized helper methods.The
getBaseParams,validatePayload, andcreateTimestampmethods are cleanly separated and easy to test independently. The TODO comment appropriately signals planned expansion.
|
🎉 This PR is included in version 1.6.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Changes
How to test
Note: Next part is to integrate into assessment code
Ref: MR-18
Summary by CodeRabbit
New Features
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.