Conversation
WalkthroughThe changes remove the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Database
participant Authorization
Caller->>Database: foreach('rules', [Query::equal('projectInternalId', ...)], callback)
activate Authorization
Note right of Authorization: Authorization::skip() wraps the operation
Database-->>Caller: For each rule document
Caller->>Database: Update rule with new deployment IDs (Authorization::skip())
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Security Scan Results for PRDocker Image Scan Results
Source Code Scan Results🎉 No vulnerabilities found! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
src/Appwrite/Platform/Modules/Functions/Workers/Builds.php (2)
1083-1091:⚠️ Potential issueCritical: Remove duplicate query condition.
The
projectInternalIdquery condition is added twice (lines 1084 and 1090). This duplication could cause database query errors or unexpected filtering behavior.Remove the duplicate query condition:
$queries = [ Query::equal("projectInternalId", [$project->getInternalId()]), Query::equal("type", ["deployment"]), Query::equal("deploymentResourceInternalId", [$resource->getInternalId()]), Query::equal('deploymentResourceType', ['function']), Query::equal('trigger', ['manual']), Query::equal('deploymentVcsProviderBranch', ['']), - Query::equal("projectInternalId", [$project->getInternalId()]) ];
1109-1117:⚠️ Potential issueCritical: Remove duplicate query condition.
The
projectInternalIdquery condition is duplicated again (lines 1110 and 1116).Remove the duplicate query condition:
$queries = [ Query::equal("projectInternalId", [$project->getInternalId()]), Query::equal("type", ["deployment"]), Query::equal("deploymentResourceInternalId", [$resource->getInternalId()]), Query::equal('deploymentResourceType', ['site']), Query::equal('trigger', ['manual']), Query::equal('deploymentVcsProviderBranch', ['']), - Query::equal("projectInternalId", [$project->getInternalId()]) ];
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/Appwrite/Platform/Modules/Compute/Base.php(0 hunks)src/Appwrite/Platform/Modules/Functions/Http/Functions/Deployment/Update.php(2 hunks)src/Appwrite/Platform/Modules/Functions/Workers/Builds.php(3 hunks)src/Appwrite/Platform/Modules/Sites/Http/Sites/Deployment/Update.php(2 hunks)
💤 Files with no reviewable changes (1)
- src/Appwrite/Platform/Modules/Compute/Base.php
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: Setup & Build Appwrite Image
- GitHub Check: scan
🔇 Additional comments (5)
src/Appwrite/Platform/Modules/Functions/Http/Functions/Deployment/Update.php (2)
14-15: LGTM! Import updated correctly.The import change from
Appwrite\QuerytoUtopia\Database\Queryaligns with the broader refactoring to use the Utopia database library's query methods.
106-120: Good implementation of direct database iteration with proper scoping.The changes correctly:
- Add
projectInternalIdfilter to ensure rules are scoped to the current project- Replace the removed
listRulesmethod with direct$dbForPlatform->foreach()iteration- Properly wrap database operations with
Authorization::skip()for system-level updatessrc/Appwrite/Platform/Modules/Sites/Http/Sites/Deployment/Update.php (1)
14-14: Consistent implementation across modules.The changes mirror those in the Functions module, maintaining consistency in how rules are queried and updated across different resource types (functions and sites).
Also applies to: 98-112
src/Appwrite/Platform/Modules/Functions/Workers/Builds.php (2)
1094-1100: LGTM! Proper implementation of direct database iteration.The replacement of
listRuleswith$dbForPlatform->forEach()is correctly implemented with appropriate query filters and authorization skipping.
1119-1124: Consistent implementation of database iteration.Both sites-related rule update operations correctly use
$dbForPlatform->foreach()with proper query filters, maintaining consistency with the refactoring pattern.Also applies to: 1176-1181
| $queries = [ | ||
| Query::equal("projectInternalId", [$project->getInternalId()]), | ||
| Query::equal("type", ["deployment"]), | ||
| Query::equal("deploymentResourceInternalId", [$resource->getInternalId()]), | ||
| Query::equal('deploymentResourceType', ['site']), | ||
| Query::equal("deploymentVcsProviderBranch", [$branchName]), | ||
| Query::equal("trigger", ['manual']), | ||
| ], $dbForPlatform, function (Document $rule) use ($dbForPlatform, $deployment) { | ||
| Query::equal("projectInternalId", [$project->getInternalId()]) | ||
| ]; |
There was a problem hiding this comment.
Critical: Remove duplicate query condition.
The projectInternalId query condition is duplicated once more (lines 1167 and 1173).
Remove the duplicate query condition:
$queries = [
Query::equal("projectInternalId", [$project->getInternalId()]),
Query::equal("type", ["deployment"]),
Query::equal("deploymentResourceInternalId", [$resource->getInternalId()]),
Query::equal('deploymentResourceType', ['site']),
Query::equal("deploymentVcsProviderBranch", [$branchName]),
Query::equal("trigger", ['manual']),
- Query::equal("projectInternalId", [$project->getInternalId()])
];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| $queries = [ | |
| Query::equal("projectInternalId", [$project->getInternalId()]), | |
| Query::equal("type", ["deployment"]), | |
| Query::equal("deploymentResourceInternalId", [$resource->getInternalId()]), | |
| Query::equal('deploymentResourceType', ['site']), | |
| Query::equal("deploymentVcsProviderBranch", [$branchName]), | |
| Query::equal("trigger", ['manual']), | |
| ], $dbForPlatform, function (Document $rule) use ($dbForPlatform, $deployment) { | |
| Query::equal("projectInternalId", [$project->getInternalId()]) | |
| ]; | |
| $queries = [ | |
| Query::equal("projectInternalId", [$project->getInternalId()]), | |
| Query::equal("type", ["deployment"]), | |
| Query::equal("deploymentResourceInternalId", [$resource->getInternalId()]), | |
| Query::equal('deploymentResourceType', ['site']), | |
| Query::equal("deploymentVcsProviderBranch", [$branchName]), | |
| Query::equal("trigger", ['manual']), | |
| ]; |
🤖 Prompt for AI Agents
In src/Appwrite/Platform/Modules/Functions/Workers/Builds.php around lines 1166
to 1174, the query condition for "projectInternalId" is duplicated. Remove the
redundant Query::equal("projectInternalId", [$project->getInternalId()]) entry
to keep only one instance of this condition in the $queries array.
✨ Benchmark results
⚡ Benchmark Comparison
|
2 participants
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
Summary by CodeRabbit