-
Notifications
You must be signed in to change notification settings - Fork 3
added new key for the resources to have a parent child relationship #96
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
|
Caution Review failedThe pull request is closed. WalkthroughA new method, Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Cache
participant Resource
Client->>Cache: add(Resource)
Cache->>Cache: getResourceCacheKey(Resource)
Cache->>Cache: Store resource status using composite key
Client->>Cache: update(Resource)
Cache->>Cache: getResourceCacheKey(Resource)
Cache->>Cache: Update resource status using composite key
Client->>Cache: remove(Resource)
Cache->>Cache: getResourceCacheKey(Resource)
Cache->>Cache: Remove resource status using composite key
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
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: 3
🔭 Outside diff range comments (1)
src/Migration/Cache.php (1)
146-148: Fix incorrect array check logic.The condition
in_array($resource->getName(), $this->cache)is incorrect. It checks if the resource name exists as a value in the cache array, but resource names are keys, not values.- if (! in_array($resource->getName(), $this->cache)) { + if (! array_key_exists($resource->getName(), $this->cache)) { $this->add($resource); }
🧹 Nitpick comments (1)
src/Migration/Cache.php (1)
77-78: Remove extra blank line.} -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Migration/Cache.php(7 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/Migration/Cache.php (4)
Learnt from: ItzNotABug
PR: #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: #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: #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: #81
File: src/Migration/Sources/CSV.php:215-233
Timestamp: 2025-07-02T06:24:50.844Z
Learning: In the utopia-php/migration codebase, invalid numeric values (like null) in parsed data are acceptable because the underlying database structure validator will catch and handle these validation errors, so explicit filtering during parsing is not required.
🔇 Additional comments (2)
src/Migration/Cache.php (2)
5-8: LGTM!The new imports are correctly added to support the hierarchical cache key generation for database resources.
95-107: LGTM!The add method correctly uses the new hierarchical cache key structure while maintaining the existing logic flow.
added new key for the resources to have a parent child relationship # Conflicts: # src/Migration/Cache.php
Issue -> Key collision for some resources leading to overwriting of resources
Solution -> having a unique key structure combining parent and child relation.
Ex-> for a document
new key -> dbid_collectionId_docId
Summary by CodeRabbit
Refactor
New Features