-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Project realtime #10764
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
base: 1.8.x
Are you sure you want to change the base?
Project realtime #10764
Conversation
📝 WalkthroughWalkthroughThis pull request introduces event queuing infrastructure for project updates across two files. The first file (app/controllers/api/projects.php) wires event queuing into the project update flow by adding an AppwriteEvent import, injecting a queue dependency, and enqueuing project update events with relevant metadata after database updates. The second file (src/Appwrite/Messaging/Adapter/Realtime.php) modifies the source of the teamId reference from the project document to the payload document when processing project-related realtime events, affecting how roles are assigned for these events. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✨ 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 |
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
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
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/controllers/api/projects.php(4 hunks)src/Appwrite/Messaging/Adapter/Realtime.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/controllers/api/projects.php (2)
src/Appwrite/Event/Event.php (2)
Event(10-654)setPayload(206-215)src/Appwrite/Utopia/Response.php (2)
Response(158-980)output(745-827)
🪛 GitHub Actions: Linter
app/controllers/api/projects.php
[error] 1-1: PSR-12: ordered_imports failed by Pint lint. Run 'vendor/bin/pint --config pint.json --verbose' to see details and fix import order.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: scan
🔇 Additional comments (5)
app/controllers/api/projects.php (4)
340-340: LGTM!The event label uses the correct pattern format for project update events.
369-370: LGTM!The dependency injection and parameter addition follow the standard Utopia framework patterns used throughout the codebase.
391-394: The code is correct as-is; no trigger() call is needed.The review comment misidentifies how
$queueForEventsworks in this framework. Unlike other queue types (e.g.,queueForDeletes,queueForMigrations),$queueForEventsis automatically triggered by the framework's middleware inapp/controllers/shared/api.php. The middleware explicitly handles event triggering by copying from$queueForEventsand calling trigger on specialized handlers:$queueForFunctions->from($queueForEvents)->trigger(),$queueForWebhooks->from($queueForEvents)->trigger(), and$queueForRealtime->from($queueForEvents)->trigger(). All other event usages in the codebase follow this same pattern of configuring parameters and payload without calling trigger() directly.
336-396: Verified: Event queuing differs across project update endpoints.The verification confirms the original observation is accurate. The main project update endpoint (
App::patch('/v1/projects/:projectId')) injectsqueueForEventsand includes event labels, while other project update endpoints—updateTeam,updateServiceStatus, andupdateOAuth2—inject onlyresponseanddbForPlatform, with no event labels or event queuing.Whether this pattern is intentional (only core metadata changes trigger realtime events) or represents incomplete implementation should be confirmed by the development team.
src/Appwrite/Messaging/Adapter/Realtime.php (1)
286-286: No action required—the change is correct.The Project response model includes
teamIdin its schema definition (src/Appwrite/Utopia/Response/Model/Project.php, line 51-55). The$payloadis generated via$response->output($project, Response::MODEL_PROJECT)and will include theteamIdattribute because it's part of the model's rules and is never filtered out. The controller also explicitly setsteamIdon the project object before response output (app/controllers/api/projects.php, line 441), ensuring the attribute is always available in the payload.The suggested defensive check is unnecessary—if
teamIdwere missing from the payload, it would indicate a broader system issue affecting the response model itself, not specific to this line.
| use Ahc\Jwt\JWT; | ||
| use Appwrite\Auth\Auth; | ||
| use Appwrite\Auth\Validator\MockNumber; | ||
| use Appwrite\Event\Event as AppwriteEvent; |
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.
Fix import order to comply with PSR-12.
The pipeline linter detected that imports are not ordered correctly according to PSR-12 standards.
Run the following command to automatically fix the import order:
vendor/bin/pint --config pint.json app/controllers/api/projects.php🤖 Prompt for AI Agents
In app/controllers/api/projects.php around line 6 the import statements are not
ordered according to PSR-12; reorder the use statements alphabetically/grouped
per PSR-12 (PHP built-ins, then third-party, then application namespaces) and/or
run the automatic fixer: vendor/bin/pint --config pint.json
app/controllers/api/projects.php to reformat the imports so the linter passes.
✨ Benchmark results
⚡ Benchmark Comparison
|
What does this PR do?
(Provide a description of what this PR does and why it's needed.)
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