Skip to content

removed redundant attribute addition making the column as the single …#150

Open
ArnabChatterjee20k wants to merge 12 commits intomultitype-dbfrom
improve-columns-setup
Open

removed redundant attribute addition making the column as the single …#150
ArnabChatterjee20k wants to merge 12 commits intomultitype-dbfrom
improve-columns-setup

Conversation

@ArnabChatterjee20k
Copy link
Contributor

@ArnabChatterjee20k ArnabChatterjee20k commented Feb 18, 2026

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

  • Chores
    • Updated storage library dependency to version 1.0.
    • Refactored database attribute and column type handling for improved type support and simplified architecture.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 18, 2026

Walkthrough

This 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)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 41.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title references 'removed redundant attribute addition making the column as the single source' which aligns with the PR's core objective of centralizing attribute conversion to the column itself, reducing redundant attribute class definitions.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch improve-columns-setup

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ArnabChatterjee20k
Copy link
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 18, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 (2)
src/Migration/Resources/Database/Columns/LongText.php (2)

88-96: getSize() and getFormat() are redundant — already defined identically in the parent Column class.

The base Column class (lines 119 and 138 in Column.php) already provides getSize(): int and getFormat(): string with 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\Database is 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments