Conversation
📝 WalkthroughWalkthroughThe change in src/Appwrite/Platform/Modules/Functions/Workers/Builds.php moves a realtime notification trigger from an early position (before readiness checks and VCS actions) to a later point, after marking the deployment as ready/activated and any activation logic. A clarifying comment is added near the new trigger. No public/exported signatures were modified. Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Appwrite/Platform/Modules/Functions/Workers/Builds.php (1)
870-893: Fix undefined $adapter in elseif and afterBuildSuccess$adapter is used in the elseif condition and later passed to afterBuildSuccess(), but it’s only defined inside the first if branch. This can emit notices and cause inconsistent behavior when detection logs are empty or resource != sites.
Apply this diff to initialize $adapter safely before the conditional:
@@ - $deployment->setAttribute('buildLogs', $logs); + $deployment->setAttribute('buildLogs', $logs); + + // Ensure $adapter is always defined for subsequent logic and callbacks + $adapter = $resource->getAttribute('adapter', ''); @@ - $adapter = $resource->getAttribute('adapter', ''); if (empty($adapter)) { $resource = $dbForProject->updateDocument('sites', $resource->getId(), new Document(['adapter' => $detection->getName(), 'fallbackFile' => $detection->getFallbackFile() ?? ''])); $deployment->setAttribute('adapter', $detection->getName()); $deployment->setAttribute('fallbackFile', $detection->getFallbackFile() ?? '');
🧹 Nitpick comments (1)
src/Appwrite/Platform/Modules/Functions/Workers/Builds.php (1)
1472-1501: Docblock completeness: missing @param for RealtimerunGitAction() accepts Realtime $queueForRealtime but the docblock omits it. Add the param for accuracy.
@@ - * @param Database $dbForPlatform + * @param Database $dbForPlatform + * @param Realtime $queueForRealtime
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Appwrite/Platform/Modules/Functions/Workers/Builds.php(1 hunks)
⏰ 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 (2)
src/Appwrite/Platform/Modules/Functions/Workers/Builds.php (2)
1187-1191: LGTM: Realtime trigger moved after resource updateThis ordering fixes the race with Console re-fetching deployment details (SER-409). Good placement after potential auto‑activation.
Please verify the VCS-connected site flow once to confirm the completion page now consistently shows deployment details.
1242-1248: Change ->foreach to ->forEach in src/Appwrite/Platform/Modules/Functions/Workers/Builds.php:1242Same file already uses ->forEach at lines 1148 and 1174; repository search did not locate the Database::forEach/foreach definition — confirm method name.
- $dbForPlatform->foreach('rules', function (Document $rule) use ($dbForPlatform, $deployment) { + $dbForPlatform->forEach('rules', function (Document $rule) use ($dbForPlatform, $deployment) {
✨ Benchmark results
⚡ Benchmark Comparison
|
Fixes SER-409
We were sending the deployment ready event to Console before the associated resource was updated with the deployment details. In edge cases (like VCS connected site deployment), this ends up not showing the deployment details in the "completion page".