Fix: v2-shims configToBlueprint not converting fields with type number #241
Conversation
|
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. 🔧 eslint
packages/v2-shims/src/plugins/config.to.blueprint.spec.tsOops! Something went wrong! :( ESLint: 8.57.1 Error [ERR_REQUIRE_ESM]: require() of ES Module /packages/v2-shims/.eslintrc.js from /node_modules/@eslint/eslintrc/dist/eslintrc.cjs not supported. WalkthroughThe changes introduce support for handling number field types across multiple files in the v2-shims package. This involves updating the type definitions in the settings interface to include 'number' as a valid field type, modifying the Changes
Sequence DiagramsequenceDiagram
participant Input as Input Config
participant Converter as configToBlueprint
participant Output as Blueprint
Input->>Converter: Field with type 'number'
Converter->>Converter: Check field type
Converter-->>Output: Set type to PlatformTypes.Num
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/v2-shims/src/plugins/config.to.blueprint.ts (2)
30-30: Remove unnecessary semicolon for consistency.The codebase doesn't use semicolons in other similar statements.
- out.type = PlatformTypes.Num; + out.type = PlatformTypes.Num
29-31: Consider using if-else consistently for type checks.The current implementation mixes
ifandelse iffor different types, which could lead to unexpected behavior if the type property is manipulated. Consider using consistent if-else statements:if (field.type === 'checkbox') { out.type = PlatformTypes.Bool - } - if (field.type === 'select') { + } else if (field.type === 'select') { out.type = PlatformTypes.Enum out.config = { options: field.options } } else if (field.type === 'number') { out.type = PlatformTypes.Num }packages/v2-shims/src/plugins/config.to.blueprint.spec.ts (1)
135-150: Consider adding edge case tests.While the basic functionality is covered, consider adding tests for:
- Mixed type scenarios (when a field has both 'select' and 'number' types)
- Invalid number configurations
- Number field with validators
Example test cases:
it('should prioritize select over number type when both are present', async () => { const input: ISettings = { type: 'Sample', fields: [{ key: 'sampleKey', label: 'sampleLabel', type: 'select' as 'select', // Type assertion needed due to union type options: [{ value: '1', label: 'One' }], }], }; const result = await configToBlueprint(input); expect(result.sheets[0].fields[0].type).toBe(PlatformTypes.Enum); }); it('should handle number field with validators', async () => { const input: ISettings = { type: 'Sample', fields: [{ key: 'sampleKey', label: 'sampleLabel', type: 'number', validators: [{ validate: 'required' }], }], }; const result = await configToBlueprint(input); expect(result.sheets[0].fields[0].type).toBe(PlatformTypes.Num); expect(result.sheets[0].fields[0].constraints).toEqual([{ type: 'required' }]); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/v2-shims/src/plugins/config.to.blueprint.spec.ts(1 hunks)packages/v2-shims/src/plugins/config.to.blueprint.ts(1 hunks)packages/v2-shims/src/types/settings.interface.ts(1 hunks)
🔇 Additional comments (3)
packages/v2-shims/src/types/settings.interface.ts (1)
154-154: LGTM! Type definition properly updated.The addition of 'number' to the union type is clean and maintains backward compatibility.
packages/v2-shims/src/plugins/config.to.blueprint.ts (1)
29-31: LGTM! Number type handling implemented correctly.The implementation properly maps the 'number' type to PlatformTypes.Num, following the existing pattern.
packages/v2-shims/src/plugins/config.to.blueprint.spec.ts (1)
135-150: LGTM! Test case properly validates number type conversion.The test case follows the established pattern and verifies the basic functionality.
1 participant
Please explain how to summarize this PR for the Changelog:
configToBlueprintfunction.PlatformTypes.NumTell code reviewer how and what to test:
Verify that the configToBlueprint function correctly processes fields with type: 'number'.
how it's currently behaving before the fix:
expected: