fix(server): use string as default field type in item import #1428
Conversation
WalkthroughThis update enhances the integration model import tests by adding a new JSON field Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner as Integration Test
participant Importer as Item Import Module
participant SchemaAnalyzer as Schema Guessing
TestRunner->>Importer: Send JSON input with new field "text2"
Importer->>SchemaAnalyzer: Process each key (handle nil values, check for "id")
SchemaAnalyzer-->>Importer: Return updated schema with field types (including "text2")
Importer-->>TestRunner: Return import result with four fields
Possibly related PRs
Suggested labels
Poem
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. 🔧 golangci-lint (1.62.2)Error: can't load config: the Go language version (go1.23) used to build golangci-lint is lower than the targeted Go version (1.24.0) ✨ 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 (
|
✅ Deploy Preview for reearth-cms canceled.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
server/internal/usecase/interactor/item_import.go (1)
531-540: Consider documenting null value handling in the CreateFieldParam struct.The implementation now correctly handles null values, but the behavior isn't documented in the struct itself.
Consider adding a comment to the
CreateFieldParamstruct to document that null values default to the "text" type. This would make the behavior clearer for future developers:return interfaces.CreateFieldParam{ ModelID: nil, SchemaID: sp.Schema().ID(), - Type: t, + Type: t, // defaults to value.TypeText for nil values Name: k, Description: lo.ToPtr("auto created by json/geoJson import"), Key: k, // type property is not supported in import TypeProperty: nil, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
server/e2e/integration_item_import_test.go(2 hunks)server/internal/usecase/interactor/item_import.go(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: ci-server / test
- GitHub Check: ci-server / lint
🔇 Additional comments (4)
server/e2e/integration_item_import_test.go (2)
216-216: Added null field test case, good enhancement!This change adds a test case for handling
nullvalues in JSON imports by including"text2": nullin the test data. This is an important edge case to test since real-world JSON often contains null fields.
269-272: Updated assertions to match schema changes, ensuring proper null field handling.The test now correctly verifies that the
text2field with a null value is:
- Properly included in the schema (4 fields instead of 3)
- Added with key "text2" alongside other keys
- Assigned the default "text" type as expected
This ensures that null fields are properly handled during schema generation.
server/internal/usecase/interactor/item_import.go (2)
507-529: Added null safety check before type evaluation.Good implementation of a null safety check that defaults to the text type for null values while still evaluating the appropriate type for non-null values.
This approach ensures that fields with null values are included in the schema with a sensible default type (text), while still correctly determining types for fields with actual values.
430-540:✅ Verification successful
Verify that all field types are handled consistently.
While the current implementation handles null values by defaulting to text type, it would be good to verify that all field types have consistent behavior with null values.
🏁 Script executed:
#!/bin/bash # Description: Check how different field types handle null values # Find any other instances where null values might be handled differently rg -A 5 "v\s*==\s*nil" --glob "*.go" server/internal/usecase/ rg -A 5 "nil\s*==" --glob "*.go" server/internal/usecase/ # Look for type conversion functions that might not handle null values properly rg -A 10 "fieldFrom|guessType|getType" --glob "*.go" server/internal/usecase/ # Find places where schema fields are created to ensure null handling is consistent rg -A 5 "NewField|CreateFieldParam" --glob "*.go" server/pkg/schema/Length of output: 28253
Field Type Null Handling Verified
I've verified that null values and type deductions are handled consistently across the code. In thefieldFromfunction, if the value (v) is nil the type defaults to text, and when non-nil, it correctly infers booleans or numbers based on thereflectkind. The null check inguessSchemaFields(skipping entries withv == nil) further confirms that the implementation intentionally avoids processing null values except when defaulting as designed. Additionally, related tests in the schema package confirm that field creation behaves as expected. No changes are needed here.
| if k == "id" { | ||
| continue | ||
| } |
There was a problem hiding this comment.
Fix for null field handling in schema detection.
This change allows the guessSchemaFields function to process keys with nil values by removing the condition that skipped them. Now it only skips the "id" field and processes all other fields, including those with null values.
This fix ensures that JSON fields with null values will be properly detected and included in the schema, providing better compatibility with diverse data sources.
Summary by CodeRabbit
Bug Fixes
Tests