Skip to content

Conversation

@ItzNotABug
Copy link
Member

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

  • (Related PR or issue)

Checklist

  • Have you read the Contributing Guidelines on issues?
  • If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 4, 2025

📝 Walkthrough

Walkthrough

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

  • Verify that $queueForEvents->setParam('projectId', $project->getId()) and ->setPayload($response->output($project, Response::MODEL_PROJECT)) are correctly populated with complete project data needed by event consumers
  • Confirm the teamId source change in Realtime.php from $project to $payload does not introduce edge cases where payload structure differs from expected project document format
  • Check that the new event label 'projects.[projectId].update' matches expected naming conventions and doesn't conflict with existing event listeners
  • Validate that both project update routes (implied by "both affected update paths") have consistent event queuing behavior
  • Ensure the inject('queueForEvents') dependency is properly registered and available in the application container

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description contains only the repository's contribution template with placeholder sections and no actual details about the changes made in the PR. Fill in the description with details about what the PR does, why it's needed, and how to test it. The changes add event queuing for project updates and modify realtime event handling.
Title check ❓ Inconclusive The title 'Project realtime' is vague and generic, using non-descriptive terms that don't clearly convey what changes were made in the changeset. Consider a more descriptive title such as 'Add event queuing for project updates' or 'Implement realtime events for project changes' to better reflect the actual implementation.
✨ 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 projects-events

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.

@github-actions
Copy link

github-actions bot commented Nov 4, 2025

Security Scan Results for PR

Docker Image Scan Results

Package Version Vulnerability Severity
binutils 2.44-r2 CVE-2025-5244 HIGH
binutils 2.44-r2 CVE-2025-5245 HIGH
libxml2 2.13.8-r0 CVE-2025-49794 CRITICAL
libxml2 2.13.8-r0 CVE-2025-49796 CRITICAL
libxml2 2.13.8-r0 CVE-2025-49795 HIGH
libxml2 2.13.8-r0 CVE-2025-6021 HIGH
pcre2 10.43-r1 CVE-2025-58050 CRITICAL
golang.org/x/crypto v0.31.0 CVE-2025-22869 HIGH
golang.org/x/oauth2 v0.24.0 CVE-2025-22868 HIGH
stdlib 1.22.10 CVE-2025-47907 HIGH
stdlib 1.22.10 CVE-2025-47912 HIGH
stdlib 1.22.10 CVE-2025-58183 HIGH
stdlib 1.22.10 CVE-2025-58186 HIGH
stdlib 1.22.10 CVE-2025-58187 HIGH
stdlib 1.22.10 CVE-2025-58188 HIGH
stdlib 1.22.10 CVE-2025-61724 HIGH

Source Code Scan Results

🎉 No vulnerabilities found!

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

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bf5aa7b and fe5c711.

📒 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 $queueForEvents works in this framework. Unlike other queue types (e.g., queueForDeletes, queueForMigrations), $queueForEvents is automatically triggered by the framework's middleware in app/controllers/shared/api.php. The middleware explicitly handles event triggering by copying from $queueForEvents and 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')) injects queueForEvents and includes event labels, while other project update endpoints—updateTeam, updateServiceStatus, and updateOAuth2—inject only response and dbForPlatform, 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 teamId in its schema definition (src/Appwrite/Utopia/Response/Model/Project.php, line 51-55). The $payload is generated via $response->output($project, Response::MODEL_PROJECT) and will include the teamId attribute because it's part of the model's rules and is never filtered out. The controller also explicitly sets teamId on 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 teamId were 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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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.

@github-actions
Copy link

github-actions bot commented Nov 4, 2025

✨ Benchmark results

  • Requests per second: 1,133
  • Requests with 200 status code: 203,935
  • P99 latency: 0.170422009

⚡ Benchmark Comparison

Metric This PR Latest version
RPS 1,133 1,160
200 203,935 208,857
P99 0.170422009 0.177325929

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.

2 participants