Conversation
Add project status attribute to platform collections and set default status in project creation
📝 WalkthroughWalkthroughAdds a new Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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)
No actionable comments were generated in the recent review. 🎉 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 |
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@app/init/constants.php`:
- Around line 393-395: The file ends with the constant PROJECT_STATUS_ACTIVE and
is missing a trailing newline which triggers PSR12 single_blank_line_at_eof;
open app/init/constants.php, locate the declaration of PROJECT_STATUS_ACTIVE and
add a single newline character at the end of the file (ensure exactly one blank
line/EOF newline) so the file ends with a newline to satisfy the linter.
In `@src/Appwrite/Platform/Modules/Projects/Http/Projects/Create.php`:
- Around line 122-123: The 'status' property is incorrectly added inside the
$auths JSON blob (so the top-level projects.status column stays null); move the
status assignment out of $auths and add it as a top-level field when creating
the project document: remove 'status' from the $auths array and include 'status'
=> PROJECT_STATUS_ACTIVE in the array passed to
$dbForPlatform->createDocument('projects', new Document([...])) (refer to
$auths, status, createDocument and the projects collection in Create.php).
🧹 Nitpick comments (3)
app/init/constants.php (1)
394-395: Consider defining additional status constants now.Only
PROJECT_STATUS_ACTIVEis defined. If the feature is meant to support disabling/suspending projects, defining the full set of expected statuses (e.g.,PROJECT_STATUS_DISABLED,PROJECT_STATUS_SUSPENDED) upfront avoids magic strings creeping in elsewhere.app/config/collections/platform.php (2)
345-355: Minor: UseID::custom('status')for consistency with other attributes.Most attributes in this collection use
ID::custom(...)for$id. This one uses a bare string. While functionally equivalent, it's inconsistent with the dominant pattern.🔧 Proposed fix
- '$id' => 'status', + '$id' => ID::custom('status'),
345-355:statusattribute hasdefault => nullbut no index — consider adding one if you plan to query/filter projects by status.If the intent is to list or filter projects by status (e.g., active vs. disabled), an index on
status(or a composite index includingstatus) will be needed. Without it, those queries will require full collection scans.
src/Appwrite/Platform/Modules/Projects/Http/Projects/Create.php
Outdated
Show resolved
Hide resolved
✨ Benchmark results
⚡ Benchmark Comparison
|
What does this PR do?
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Screenshots may also be helpful.)
Related PRs and Issues
Checklist