-
Notifications
You must be signed in to change notification settings - Fork 3
Add: support for array attributes #81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe update enhances the CSV document export logic to correctly handle attributes that are arrays but not relationships. It introduces tracking for such attributes, updates the parsing logic to convert CSV values into properly typed arrays, and refines typecasting and empty value handling during the import process. Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Migration/Sources/CSV.php (1)
109-263: Consider adding documentation for the CSV array format.The implementation would benefit from clear documentation about:
- The expected CSV format for array values (comma-separated, escaping rules)
- How empty arrays are represented
- Examples of valid array formats for different data types
This will help users understand the requirements and limitations of the CSV import/export functionality.
Would you like me to help create documentation comments or a separate documentation file explaining the CSV format requirements?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Migration/Sources/CSV.php(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ItzNotABug
PR: utopia-php/migration#80
File: src/Migration/Sources/Supabase.php:300-308
Timestamp: 2025-06-28T09:47:58.757Z
Learning: In the utopia-php/migration codebase, during the terminology swap from Collection/Attribute/Document to Table/Column/Row, the user ItzNotABug prefers to keep the existing query logic unchanged even if it becomes semantically incorrect with the new naming. The focus is purely on resource type renaming, not on fixing logical issues that become apparent after the terminology change.
Learnt from: ItzNotABug
PR: utopia-php/migration#80
File: src/Migration/Resources/Database/Row.php:60-60
Timestamp: 2025-06-28T09:45:36.026Z
Learning: In the utopia-php/migration codebase, the `fromArray` method is not used on Row objects, so mismatches between `jsonSerialize()` output keys and `fromArray()` input expectations for Row class are not problematic.
Learnt from: ItzNotABug
PR: utopia-php/migration#80
File: src/Migration/Sources/Appwrite.php:843-851
Timestamp: 2025-06-28T09:47:08.333Z
Learning: In the utopia-php/migration codebase, during the terminology swap from Collection/Attribute/Document to Table/Column/Row, the class constructors and method parameters use the new terminology (like "relatedTable"), but the underlying data structures and API responses still use the legacy keys (like "relatedCollection"). This is an intentional design pattern to allow gradual migration while maintaining compatibility with existing data sources.
src/Migration/Sources/CSV.php (3)
Learnt from: ItzNotABug
PR: utopia-php/migration#80
File: src/Migration/Resources/Database/Row.php:60-60
Timestamp: 2025-06-28T09:45:36.026Z
Learning: In the utopia-php/migration codebase, the `fromArray` method is not used on Row objects, so mismatches between `jsonSerialize()` output keys and `fromArray()` input expectations for Row class are not problematic.
Learnt from: ItzNotABug
PR: utopia-php/migration#80
File: src/Migration/Sources/Appwrite.php:843-851
Timestamp: 2025-06-28T09:47:08.333Z
Learning: In the utopia-php/migration codebase, during the terminology swap from Collection/Attribute/Document to Table/Column/Row, the class constructors and method parameters use the new terminology (like "relatedTable"), but the underlying data structures and API responses still use the legacy keys (like "relatedCollection"). This is an intentional design pattern to allow gradual migration while maintaining compatibility with existing data sources.
Learnt from: ItzNotABug
PR: utopia-php/migration#80
File: src/Migration/Sources/Supabase.php:300-308
Timestamp: 2025-06-28T09:47:58.757Z
Learning: In the utopia-php/migration codebase, during the terminology swap from Collection/Attribute/Document to Table/Column/Row, the user ItzNotABug prefers to keep the existing query logic unchanged even if it becomes semantically incorrect with the new naming. The focus is purely on resource type renaming, not on fixing logical issues that become apparent after the terminology change.
🧬 Code Graph Analysis (1)
src/Migration/Sources/CSV.php (1)
src/Migration/Resources/Database/Attribute.php (2)
isArray(111-114)Attribute(8-149)
🔇 Additional comments (4)
src/Migration/Sources/CSV.php (4)
138-169: LGTM! Proper handling of array attribute metadata.The code correctly extracts attribute properties and identifies array attributes that are not relationships. The separation between
$arrayKeysand$manyToManyKeysprovides a clean way to handle different types of array data during CSV parsing.
171-171: Good refinement of the type checking logic.Using
isset()instead ofempty()is more precise and avoids skipping attributes with falsy but valid type values.Also applies to: 197-199
201-213: Improved many-to-many relationship parsing.The refined logic properly handles empty values and filters out empty strings after trimming, resulting in cleaner array data.
235-242: Good addition of empty string guard.The check prevents assigning null or false values for empty CSV fields, maintaining consistency with the array handling logic.
Summary by CodeRabbit
New Features
Bug Fixes