removed redundant attribute addition making the column as the single …#150
removed redundant attribute addition making the column as the single …#150ArnabChatterjee20k wants to merge 12 commits intomultitype-dbfrom
Conversation
Text attributes
chore: bump storage lib
WalkthroughThis PR updates the utopia-php/storage dependency from 0.18.* to 1.0.*, removes multiple Attribute-specific classes (Decimal, Email, IP, Integer, Line, ObjectType, Point, Polygon, Relationship, URL, Vector), introduces new Column-based classes (LongText, MediumText, RegularText, Varchar), adds a GenericAttribute class for generic type handling, expands Column type constants, centralizes attribute retrieval through Column::getAttribute() and GenericAttribute, and refactors the Appwrite source and destination classes to align with the new architecture. Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/Migration/Resources/Database/Columns/LongText.php (2)
88-96:getSize()andgetFormat()are redundant — already defined identically in the parentColumnclass.The base
Columnclass (lines 119 and 138 inColumn.php) already providesgetSize(): intandgetFormat(): stringwith the same implementation (return $this->size;/return $this->format;). These overrides can be removed. The same applies to MediumText, RegularText, and Varchar.🧹 Remove redundant overrides
- public function getSize(): int - { - return $this->size; - } - - public function getFormat(): string - { - return $this->format; - } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Migration/Resources/Database/Columns/LongText.php` around lines 88 - 96, Remove the redundant getSize() and getFormat() overrides from LongText (and similarly from MediumText, RegularText, Varchar) since the base Column class already implements public function getSize(): int and public function getFormat(): string returning $this->size and $this->format; simply delete the duplicate methods from these child classes so they inherit the parent implementations (ensure the classes still compile and run the existing tests after removal).
5-5: Unused import:Utopia\Database\Databaseis not referenced in this file.This import appears to be a leftover from the previous Attribute-based hierarchy. The same unused import exists in all four new Column subclasses (LongText, MediumText, RegularText, Varchar).
🧹 Remove unused import
-use Utopia\Database\Database;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Migration/Resources/Database/Columns/LongText.php` at line 5, Remove the unused import Utopia\Database\Database from the file containing the LongText class (and likewise from MediumText, RegularText, and Varchar files) since it is not referenced; locate the top-of-file use statement "use Utopia\Database\Database;" in each of the Column subclasses (LongText, MediumText, RegularText, Varchar) and delete that line, then run the project linter/typecheck to confirm no remaining references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@composer.json`:
- Line 30: The composer dependency "utopia-php/storage" uses a non-existent
version constraint "1.0.*"; update the composer.json entry for the package
"utopia-php/storage" to a valid published version (e.g., "0.20.0" or a semver
constraint like "^0.20" or ">=0.20 <1.0") and then run composer update to
refresh the lockfile and vendor files so the project installs a real release.
---
Nitpick comments:
In `@src/Migration/Resources/Database/Columns/LongText.php`:
- Around line 88-96: Remove the redundant getSize() and getFormat() overrides
from LongText (and similarly from MediumText, RegularText, Varchar) since the
base Column class already implements public function getSize(): int and public
function getFormat(): string returning $this->size and $this->format; simply
delete the duplicate methods from these child classes so they inherit the parent
implementations (ensure the classes still compile and run the existing tests
after removal).
- Line 5: Remove the unused import Utopia\Database\Database from the file
containing the LongText class (and likewise from MediumText, RegularText, and
Varchar files) since it is not referenced; locate the top-of-file use statement
"use Utopia\Database\Database;" in each of the Column subclasses (LongText,
MediumText, RegularText, Varchar) and delete that line, then run the project
linter/typecheck to confirm no remaining references.
Problem -> Adding new column was getting extremely hard as we have to add attribute along with the new column plus addition in other places as well
Solution -> Going with a centralized approch of attribute conversion form column itself whenever required
Summary by CodeRabbit