Conversation
WalkthroughThis update introduces several changes across the codebase, including stricter constraints on team membership role changes and deletions, removal of encrypted string attribute logic, updates to SDK and runtime versions, Docker image tag bumps, error page branding removal, dependency version updates, and improvements to domain generation for VCS previews. Various tests and migrations are also updated to reflect these changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant DB
%% Team Membership Role Change/Deletion Flow
Client->>API: Request to update/delete team membership
API->>DB: Fetch membership and team info
API->>DB: Count owners in team
alt Only one owner and user is owner
API-->>Client: Error (MEMBERSHIP_DOWNGRADE_PROHIBITED or MEMBERSHIP_DELETION_PROHIBITED)
else
API->>DB: Update/Delete membership
API-->>Client: Success
end
sequenceDiagram
participant API
participant DB
%% VCS Preview Domain Generation
API->>API: Truncate branch/commit name, hash if needed
API->>API: Construct domain with hash suffix
API->>DB: Create or update deployment rule with new domain
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (4)
✨ 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! |
commit: |
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
app/controllers/api/databases.php (1)
1350-1375: 🛠️ Refactor suggestionConsider broader implications of encryption feature removal.
The removal of encrypted string attribute functionality represents a significant breaking change. Consider the following:
- Breaking Change Impact: Existing applications using encrypted attributes may be affected
- Migration Path: Users with existing encrypted attributes may need guidance
- Documentation: API documentation should be updated to reflect the removal
- Security Review: Ensure the removal doesn't inadvertently expose previously encrypted data
Would you like me to help identify other areas of the codebase that might need updates related to this encryption removal?
🧹 Nitpick comments (3)
src/Appwrite/Platform/Modules/Functions/Workers/Builds.php (1)
1135-1142: Consider extracting domain generation logic to a shared utility.This domain generation logic is duplicated from
vcs.php. Consider extracting it to a shared utility function to maintain consistency and follow DRY principles.Create a utility class or function like
VcsDomainGenerator::generateBranchDomain($branchName, $resourceId, $projectId)that can be reused across both files. This would ensure consistent behavior and make future changes easier to maintain.app/config/errors.php (1)
396-405: Error entries follow existing pattern
The added definitions correctly reference theExceptionconstants, use consistent HTTP status codes, and fit the surrounding structure. Consider adding unit tests to cover these new error codes and updating the API documentation/error reference to include their descriptions.app/controllers/api/teams.php (1)
1100-1102: Consider using a more descriptive variable name.The variable name
$isCurrentUserAnOwnermight be misleading as it checks if the current user is modifying their own membership, not whether they are an owner. Consider renaming it to better reflect its purpose.-// Is the role change being requested by the user on their own membership? -$isCurrentUserAnOwner = $user->getInternalId() === $membership->getAttribute('userInternalId'); +// Is the role change being requested by the user on their own membership? +$isModifyingOwnMembership = $user->getInternalId() === $membership->getAttribute('userInternalId');
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (16)
app/config/specs/open-api3-1.7.x-client.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-1.7.x-console.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-1.7.x-server.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-latest-client.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-latest-console.jsonis excluded by!app/config/specs/**app/config/specs/open-api3-latest-server.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-1.7.x-client.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-1.7.x-console.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-1.7.x-server.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-latest-client.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-latest-console.jsonis excluded by!app/config/specs/**app/config/specs/swagger2-latest-server.jsonis excluded by!app/config/specs/**composer.lockis excluded by!**/*.lockdocs/examples/1.7.x/console-cli/examples/databases/upsert-document.mdis excluded by!docs/examples/**docs/examples/1.7.x/console-cli/examples/databases/upsert-documents.mdis excluded by!docs/examples/**docs/sdks/flutter/CHANGELOG.mdis excluded by!docs/sdks/**
📒 Files selected for processing (37)
.env(1 hunks).github/workflows/sdk-preview.yml(1 hunks)README-CN.md(3 hunks)README.md(3 hunks)app/cli.php(1 hunks)app/config/collections/common.php(0 hunks)app/config/errors.php(1 hunks)app/config/platforms.php(4 hunks)app/config/template-runtimes.php(2 hunks)app/controllers/api/databases.php(5 hunks)app/controllers/api/storage.php(7 hunks)app/controllers/api/teams.php(4 hunks)app/controllers/api/vcs.php(2 hunks)app/init/constants.php(1 hunks)app/init/database/filters.php(1 hunks)app/views/general/404.phtml(0 hunks)app/views/general/error.phtml(0 hunks)app/views/install/compose.phtml(2 hunks)composer.json(2 hunks)docker-compose.yml(2 hunks)src/Appwrite/Extend/Exception.php(1 hunks)src/Appwrite/Migration/Migration.php(1 hunks)src/Appwrite/Migration/Version/V22.php(1 hunks)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(4 hunks)src/Appwrite/Platform/Modules/Sites/Http/Deployments/Create.php(1 hunks)src/Appwrite/Platform/Modules/Sites/Http/Sites/Deployment/Update.php(2 hunks)src/Appwrite/Platform/Tasks/ScheduleBase.php(3 hunks)src/Appwrite/Platform/Tasks/ScheduleFunctions.php(1 hunks)src/Appwrite/Platform/Tasks/ScheduleMessages.php(1 hunks)src/Appwrite/Utopia/Response/Model/AttributeString.php(0 hunks)tests/e2e/Services/Databases/DatabasesBase.php(1 hunks)tests/e2e/Services/Databases/DatabasesCustomServerTest.php(1 hunks)tests/e2e/Services/Sites/SitesCustomServerTest.php(2 hunks)tests/e2e/Services/Teams/TeamsBase.php(1 hunks)tests/e2e/Services/Teams/TeamsConsoleClientTest.php(6 hunks)
💤 Files with no reviewable changes (5)
- src/Appwrite/Utopia/Response/Model/AttributeString.php
- app/config/collections/common.php
- src/Appwrite/Platform/Modules/Compute/Base.php
- app/views/general/error.phtml
- app/views/general/404.phtml
🧰 Additional context used
🧬 Code Graph Analysis (6)
src/Appwrite/Platform/Modules/Functions/Http/Functions/Deployment/Update.php (1)
src/Appwrite/Utopia/Response/Model/Document.php (1)
Document(8-92)
src/Appwrite/Platform/Tasks/ScheduleBase.php (2)
app/realtime.php (1)
getProjectDB(76-122)src/Appwrite/Utopia/Response/Model/Document.php (1)
Document(8-92)
app/config/errors.php (1)
src/Appwrite/Extend/Exception.php (1)
Exception(7-408)
src/Appwrite/Platform/Modules/Sites/Http/Sites/Deployment/Update.php (1)
src/Appwrite/Utopia/Response/Model/Document.php (1)
Document(8-92)
src/Appwrite/Migration/Version/V22.php (1)
src/Appwrite/Platform/Workers/Databases.php (1)
deleteIndex(448-487)
src/Appwrite/Platform/Modules/Sites/Http/Deployments/Create.php (1)
src/Appwrite/Vcs/Comment.php (1)
isEmpty(26-29)
⏰ Context from checks skipped due to timeout of 90000ms (189)
- GitHub Check: E2E Service Test (Migrations)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E Shared Mode Service Test (Dev Keys) (Shared V1)
- GitHub Check: E2E Service Test (Dev Keys)
- GitHub Check: E2E General Test
- GitHub Check: Benchmark
- GitHub Check: E2E Service Test (Migrations)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E Shared Mode Service Test (Dev Keys) (Shared V1)
- GitHub Check: E2E Service Test (Dev Keys)
- GitHub Check: E2E General Test
- GitHub Check: Benchmark
- GitHub Check: E2E Service Test (Migrations)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E Shared Mode Service Test (Dev Keys) (Shared V1)
- GitHub Check: E2E Service Test (Dev Keys)
- GitHub Check: E2E General Test
- GitHub Check: Benchmark
- GitHub Check: E2E Service Test (Migrations)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E Service Test (Dev Keys)
- GitHub Check: E2E General Test
- GitHub Check: Benchmark
- GitHub Check: E2E Service Test (Webhooks)
- GitHub Check: E2E Service Test (Migrations)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E Service Test (Dev Keys)
- GitHub Check: E2E General Test
- GitHub Check: Benchmark
- GitHub Check: E2E Service Test (Webhooks)
- GitHub Check: E2E Service Test (Migrations)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E General Test
- GitHub Check: Benchmark
- GitHub Check: E2E Service Test (Webhooks)
- GitHub Check: E2E Service Test (Migrations)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E General Test
- GitHub Check: Benchmark
- GitHub Check: E2E Service Test (Webhooks)
- GitHub Check: E2E Service Test (Migrations)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Databases)
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E General Test
- GitHub Check: Benchmark
- GitHub Check: E2E Service Test (Projects)
- GitHub Check: E2E Service Test (Webhooks)
- GitHub Check: E2E Service Test (Migrations)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Databases)
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E General Test
- GitHub Check: Benchmark
- GitHub Check: E2E Service Test (Projects)
- GitHub Check: E2E Service Test (Webhooks)
- GitHub Check: E2E Service Test (Migrations)
- GitHub Check: E2E Service Test (Messaging)
- GitHub Check: E2E Service Test (Sites)
- GitHub Check: E2E Service Test (Proxy)
- GitHub Check: E2E Service Test (Realtime)
- GitHub Check: E2E Service Test (Teams)
- GitHub Check: E2E Service Test (FunctionsSchedule)
- GitHub Check: E2E Service Test (Databases)
- GitHub Check: E2E Service Test (Storage)
- GitHub Check: E2E Service Test (Avatars)
- GitHub Check: E2E Service Test (Console)
- GitHub Check: E2E Service Test (GraphQL)
- GitHub Check: E2E Service Test (Health)
- GitHub Check: E2E Service Test (Locale)
- GitHub Check: E2E Service Test (Functions)
- GitHub Check: E2E Service Test (Account)
- GitHub Check: E2E General Test
- GitHub Check: Benchmark
🔇 Additional comments (60)
tests/e2e/Services/Sites/SitesCustomServerTest.php (2)
1449-1449: Assert removal of branding matches UI updates.The test is now correctly verifying that the "Powered by" footer is no longer present on non-SPA 404 pages, aligning with the removal of the
<div class="brand">section from the templates.
1505-1505: Align testSiteStaticSPA with branding removal.Updating the assertion to confirm the absence of "Powered by" on SPA 404 pages reflects the UI cleanup and ensures tests stay in sync with the new error page markup.
src/Appwrite/Platform/Tasks/ScheduleMessages.php (1)
43-43: LGTM! Good cleanup of unused variable.The removal of
$poolsfrom the closure'suseclause is correct as the variable is not referenced anywhere within the closure (lines 44-61). This improves code cleanliness by eliminating unused variable captures.src/Appwrite/Platform/Tasks/ScheduleFunctions.php (1)
79-79: LGTM! Consistent cleanup of unused variable.The removal of
$poolsfrom the closure'suseclause is appropriate as the variable is not used anywhere within the coroutine logic (lines 80-105). This change aligns with the broader cleanup pattern across scheduling task files.src/Appwrite/Platform/Tasks/ScheduleBase.php (4)
82-82: LGTM! Method calls updated consistently with signature change.The removal of the
$poolsparameter fromcollectSchedulesmethod calls aligns perfectly with the updated method signature. The calls remain functionally correct.Also applies to: 91-91
88-88: LGTM! Timer callback cleanup.The removal of
$poolsfrom the timer callback'suseclause is correct as the variable is not referenced within the callback logic.
106-106: LGTM! Method signature cleanup.The removal of the unused
Group $poolsparameter from thecollectSchedulesmethod signature is a good cleanup. The parameter was not used anywhere within the method implementation.
118-118: LGTM! Closure variable cleanup.The removal of
$poolsfrom the$getScheduleclosure'suseclause is appropriate as the variable is not referenced anywhere within the closure implementation (lines 119-136).src/Appwrite/Platform/Modules/Sites/Http/Deployments/Create.php (2)
350-360: Inconsistent summary: Changes don't align with PR objectives.The PR objectives mention removing the
StatsUsageDumpcomponent, but this file shows changes to deployment rules management instead.Likely an incorrect or invalid review comment.
285-295: Good: Consistent rules document structure across both creation paths.The enriched metadata structure for rules documents is consistently applied in both the complete upload path (lines 285-295) and the chunked upload path (lines 350-360). This ensures uniform rule creation regardless of the upload method.
Also applies to: 350-360
src/Appwrite/Platform/Modules/Sites/Http/Sites/Deployment/Update.php (2)
99-105: LGTM! Query filters properly target manual deployment rules.The updated query construction correctly:
- Uses array syntax for the
triggerfilter- Filters for manual deployments (empty VCS branch)
- Scopes to the current project using
projectInternalId
107-112: Good refactoring: Direct database iteration with proper authorization handling.The replacement of
listRuleswith direct database iteration provides better transparency and control. The nestedAuthorization::skipensures both the iteration and document updates bypass authorization checks appropriately.src/Appwrite/Platform/Modules/Functions/Http/Functions/Deployment/Update.php (1)
107-112: Excellent: Consistent implementation pattern across Sites and Functions.The changes mirror those in the Sites deployment update, ensuring uniform behavior for deployment rule updates across different resource types. This consistency improves maintainability.
Also applies to: 115-120
app/controllers/api/vcs.php (1)
333-333: LGTM!The commit hash truncation to 16 characters is appropriate and provides sufficient uniqueness for preview domains while keeping URLs manageable.
composer.json (1)
64-64: Dependency version constraint updates approvedThe version bumps for
utopia-php/messaging,utopia-php/queue,dragonmantank/cron-expression,phpmailer/phpmailer,chillerlan/php-qrcode,adhocore/jwt, and the dev dependencyappwrite/sdk-generatorare consistent with intended minor/patch upgrades. Ensure that existing integration and compatibility tests pass against these new versions.Also applies to: 70-70, 79-82, 89-89
README.md (1)
81-81: Consistent Docker image tag updatedAll three installation command examples have been updated from
1.7.3to1.7.4, aligning with the bumped stableAPP_VERSION_STABLE. This maintains consistency across Unix, CMD, and PowerShell instructions.Also applies to: 93-93, 103-103
.env (1)
88-88:Details
✅ Verification successful
Updated Flutter runtime version in SITES_RUNTIMES
The
_APP_SITES_RUNTIMESentry now includesflutter-3.32. Please verify that this runtime version is supported by your template configuration (app/config/template-runtimes.php).
🏁 Script executed:
#!/bin/bash # Verify that flutter-3.32 is present in the supported template runtimes rg --color=always "flutter-3\\.32" -n app/config/template-runtimes.phpLength of output: 71
🏁 Script executed:
#!/bin/bash # List any flutter runtime entries in the template configuration rg --color=always "flutter" -n app/config/template-runtimes.phpLength of output: 125
🏁 Script executed:
#!/bin/bash # Show context around the flutter entry in template-runtimes.php to verify supported versions rg --color=always -n "flutter" -C 5 app/config/template-runtimes.phpLength of output: 426
Flutter-3.32 runtime version is supported
The
flutter-3.32runtime you added is already declared in the template configuration:• app/config/template-runtimes.php (lines 39–42):
'FLUTTER' => [ 'name' => 'flutter', 'versions' => ['3.32', '3.24'] ]No further action required.
src/Appwrite/Migration/Migration.php (1)
91-91: New migration version mapping addedThe
'1.7.4' => 'V22'entry correctly maps the new application version to the existingV22migration class, ensuring the migration runner recognizes version1.7.4.tests/e2e/Services/Databases/DatabasesBase.php (1)
301-301: Remove stale encryption flag assertion
Theencryptproperty has been removed from the string attribute response model, so dropping this assertion intestCreateAttributesreflects the updated behavior.README-CN.md (1)
75-75: LGTM: Consistent Docker image version updateThe Docker image version has been properly updated from
1.7.3to1.7.4across all installation commands (Unix, Windows CMD, and PowerShell). This ensures consistency in the installation documentation.Also applies to: 87-87, 97-97
tests/e2e/Services/Teams/TeamsBase.php (1)
67-67: LGTM: Improved error type specificityThe test assertion has been updated to expect the more specific error type
'membership_downgrade_prohibited'instead of the generic'general_argument_invalid'. This reflects improved error handling in the business logic, providing more meaningful error messages for membership-related operations..github/workflows/sdk-preview.yml (1)
50-50:Details
❓ Verification inconclusive
Verify the hardcoded working directory path
The working directory has been changed from a dynamic path
./app/sdks/${{ steps.set-sdk.outputs.platform }}-${{ steps.set-sdk.outputs.sdk_type }}to a static path./app/sdks/console-web. This removes the flexibility of the workflow to build different SDK types based on the platform input.The workflow still includes logic to determine platform and SDK type (steps.set-sdk), but now ignores these outputs when setting the working directory. This could break the workflow for server platform builds.
Please verify if this change is intentional. If the workflow should only support console-web builds, consider:
- Removing the unused platform input and SDK type logic
- Updating the workflow description to reflect the limitation
If the workflow should remain flexible, revert to the dynamic path or ensure the static path works for all intended platforms.
🏁 Script executed:
#!/bin/bash set -e echo "Lines 40-60 of .github/workflows/sdk-preview.yml:" sed -n '40,60p' .github/workflows/sdk-preview.yml || true echo echo "Occurrences of 'set-sdk' in sdk-preview.yml:" grep -R "set-sdk" -n .github/workflows/sdk-preview.yml || trueLength of output: 1290
🏁 Script executed:
#!/bin/bash set -e echo "----- Lines 1-80 of .github/workflows/sdk-preview.yml -----" sed -n '1,80p' .github/workflows/sdk-preview.yml || true echo echo "----- Context around set-sdk step (lines 20-50) -----" sed -n '20,50p' .github/workflows/sdk-preview.yml || trueLength of output: 3010
Align Build step working-directory with dynamic SDK path
The Build and Publish SDK step at
.github/workflows/sdk-preview.yml:48–50is using a hardcodedworking-directory: ./app/sdks/console-webwhile the previous steps still compute and pull into
./app/sdks/${{ steps.set-sdk.outputs.platform }}-${{ steps.set-sdk.outputs.sdk_type }}This makes the workflow inconsistent and will break manual dispatches (e.g.
platform=server) since onlyconsole-webis built.Please verify the intended scope of this workflow:
- If it should remain flexible for all platforms:
• Update toworking-directory: ./app/sdks/${{ steps.set-sdk.outputs.platform }}-${{ steps.set-sdk.outputs.sdk_type }}- If it’s now console-only:
• Remove the unusedset-sdklogic and dynamic paths in earlier steps
• Update the workflow trigger/description to reflect console-web exclusivityapp/config/template-runtimes.php (1)
17-17: LGTM: Runtime version updatesThe addition of newer runtime versions is appropriate:
- Dart '3.8' added to the beginning of the versions array
- Flutter '3.32' added to the beginning of the versions array
These updates ensure the latest runtime versions are available and prioritized in the configuration.
Also applies to: 41-41
src/Appwrite/Migration/Version/V22.php (1)
153-155: LGTM! Index deletion logic is correctly implemented.The change from creating to deleting the
_key_rolesindex in the memberships collection is consistent and the warning message has been appropriately updated to reflect the deletion operation. The method call uses the correct parameters (collection ID and index key) as expected by thedeleteIndexmethod.app/cli.php (1)
287-287:Details
❓ Verification inconclusive
Verify the impact of enabling TCP hooks in coroutines.
The change from excluding TCP hooks to enabling all coroutine hooks (
SWOOLE_HOOK_ALL) is significant. Please ensure that:
- There are no compatibility issues with existing functionality that relied on TCP hooks being disabled
- Performance implications have been considered and tested
- This change aligns with the specific Swoole version being used
🏁 Script executed:
#!/bin/bash # Description: Check if there are any specific configurations or comments about TCP hook exclusions # Search for any references to TCP hooks or SWOOLE_HOOK configurations rg -i "swoole.*hook|tcp.*hook" --type php -A 3 -B 3 # Look for any documentation or comments about coroutine hook usage rg -i "coroutine.*hook|hook.*tcp" --type php -A 2 -B 2Length of output: 2781
Confirm TCP Hooks in CLI & Realtime Entry Points
The
Runtime::enableCoroutine(SWOOLE_HOOK_ALL)calls in bothapp/cli.php(line 287) andapp/realtime.phpnow include TCP hook support, whereas the worker classes still explicitly exclude TCP hooks viaRuntime::setHookFlags(SWOOLE_HOOK_ALL ^ SWOOLE_HOOK_TCP);Please verify that:
- Enabling TCP hooks in the CLI and realtime contexts is intentional and does not break existing TCP-based operations.
- You’ve run performance tests on your CLI commands and realtime processes with these flags enabled.
- Your project’s Swoole version supports
Runtime::enableCoroutine()with the fullSWOOLE_HOOK_ALLflag set.Affected locations:
- app/cli.php:
Runtime::enableCoroutine(SWOOLE_HOOK_ALL)- app/realtime.php:
Runtime::enableCoroutine(SWOOLE_HOOK_ALL)- Worker files remain on
SWOOLE_HOOK_ALL ^ SWOOLE_HOOK_TCPapp/init/constants.php (1)
40-41: LGTM! Version and cache buster updates are appropriate.The version bump to
1.7.4and cache buster increment from4319to4320are standard practices for a new release. The cache buster increment will ensure proper invalidation of cached assets in browsers.src/Appwrite/Extend/Exception.php (1)
127-128: LGTM! New membership error constants are well-defined.The new error constants
MEMBERSHIP_DELETION_PROHIBITEDandMEMBERSHIP_DOWNGRADE_PROHIBITEDfollow the established naming convention and are properly placed within the Membership section. These will provide more specific error handling for membership management operations.docker-compose.yml (2)
954-954:Details
✅ Verification successful
Verify the executor image version update.
The openruntimes-executor image has been updated from
0.7.16to0.7.20. Please ensure this version exists and is compatible with the current Appwrite setup.
🏁 Script executed:
#!/bin/bash # Verify that the new executor image version exists curl -s "https://registry.hub.docker.com/v2/repositories/openruntimes/executor/tags/" | jq '.results[] | select(.name == "0.7.20")'Length of output: 2344
Executor image version validated
- docker-compose.yml (line 954) now uses
openruntimes/executor:0.7.20.- Docker Hub confirms the
0.7.20tag exists, and as a patch‐level bump from0.7.16, it remains compatible with the current Appwrite setup.Approving this change.
216-216:Details
✅ Verification successful
Verify the console image version update.
The appwrite-console image has been updated from
6.0.11to6.0.13. Please ensure this version exists and has been tested for compatibility.
🏁 Script executed:
#!/bin/bash # Verify that the new console image version exists curl -s "https://registry.hub.docker.com/v2/repositories/appwrite/console/tags/" | jq '.results[] | select(.name == "6.0.13")'Length of output: 126
I didn’t find the
6.0.13tag in the default page—let’s pull more results to be sure:
🏁 Script executed:
#!/bin/bash # Fetch up to 100 tags to verify existence of 6.0.13 curl -s "https://registry.hub.docker.com/v2/repositories/appwrite/console/tags?page_size=100" \ | jq '.results[] | select(.name == "6.0.13")'Length of output: 2355
Console image tag
6.0.13exists and is active
- Confirmed on Docker Hub (last pushed 2025-05-25, status: active).
No further verification needed—update looks good.
app/views/install/compose.phtml (2)
180-180: Consistent console image version update.The console image version update to
6.0.13is consistent with the change indocker-compose.yml, maintaining synchronization between development and production configurations.
867-867: Consistent executor image version update.The executor image version update to
0.7.20maintains consistency with the development docker-compose configuration.app/config/errors.php (1)
396-405: Unrelated to PR objective
These new membership error definitions (MEMBERSHIP_DELETION_PROHIBITEDandMEMBERSHIP_DOWNGRADE_PROHIBITED) appear to be outside the scope of the “Fix usage queues” PR, which is aimed at removing theStatsUsageDumpcomponent. Please confirm whether these entries were intended for this branch or included by mistake.tests/e2e/Services/Databases/DatabasesCustomServerTest.php (1)
698-700: Inconsistency between PR objectives and actual changes detected.The PR objectives state this is about "removing the
StatsUsageDumpcomponent", but the AI summary indicates this change removes encryption size validation logic and encryption flag verification. Theencryptparameter is still being used on line 698, but according to the summary, related validation checks were removed.This raises several concerns:
- The test method
testCreateEncryptedAttributemay no longer properly validate encryption functionality- The method name suggests it tests encryption, but if validation logic was removed, the test coverage may be insufficient
- There's no visibility into what specific assertions or checks were removed
Please verify what specific encryption validation logic was removed from this test method and whether the test still adequately covers encryption functionality. Consider running the following script to check for any remaining encryption-related assertions:
#!/bin/bash # Description: Search for encryption-related test assertions and validation logic # Expected: Find any remaining encryption validation in the test # Search for encryption-related assertions in this test file rg -A 10 -B 5 "encrypt|Encrypt" tests/e2e/Services/Databases/DatabasesCustomServerTest.php # Search for encryption-related validation in all test files rg -A 5 "assertContains.*encrypt|assertEquals.*encrypt|assertTrue.*encrypt" tests/e2e/Services/Databases/ # Check if there are other encryption test methods ast-grep --pattern $'function $_($$$) { $$$ encrypt $$$ }'Likely an incorrect or invalid review comment.
app/controllers/api/databases.php (4)
1371-1371: LGTM - Encryption logic cleanup.The empty line indicates proper removal of encryption-related code. This aligns with the objective to remove encrypted string attribute restrictions.
2114-2114: LGTM - Proper cleanup of removed functionality.The empty lines indicate clean removal of encryption-related attribute handling code, which aligns with the PR objectives.
Also applies to: 2134-2134
4584-4584: Inconsistency between summary and implementation.The AI summary states that the
documentsparameter was made optional, but the code showsfalsefor the optional parameter (meaning it's required). This discrepancy needs clarification.#!/bin/bash # Description: Check the intended behavior for the documents parameter # Expected: Clarification on whether documents should be optional or required # Search for other bulk document endpoints to compare parameter definitions rg -A 3 -B 3 "param.*documents.*ArrayList" --type php # Look for any documentation or tests that might indicate the intended behavior rg -A 5 -B 5 "documents.*optional|optional.*documents" --type phpLikely an incorrect or invalid review comment.
1350-1350:Details
✅ Verification successful
Verify complete removal of plan-based restrictions.
The removal of the
planparameter injection suggests plan-based restrictions on encrypted string attributes have been eliminated. Ensure this change is intentional and complete throughout the codebase.
🏁 Script executed:
#!/bin/bash # Description: Check for any remaining references to plan-based encryption restrictions # Expected: No remaining references to plan validation for encryption # Search for any remaining plan-based encryption validation rg -A 5 -B 5 "plan.*encrypt|encrypt.*plan" --type php # Search for any remaining APP_DATABASE_ENCRYPT_SIZE_MIN references rg "APP_DATABASE_ENCRYPT_SIZE_MIN" --type php # Search for plan parameter usage in string attribute creation rg -A 10 "createStringAttribute.*plan" --type phpLength of output: 153
Plan-parameter removal verified complete
The ripgrep search for any remaining references to
plan-based encryption restrictions (includingplan.*encrypt,APP_DATABASE_ENCRYPT_SIZE_MIN, andcreateStringAttribute.*plan) returned no matches. This indicates theplanparameter has been fully removed from encrypted-string attribute logic.No further changes appear necessary. Please ensure existing tests cover the altered behavior.
app/controllers/api/storage.php (14)
1007-1007: Good improvement for configurability!Using
System::getEnv('_APP_STORAGE_PREVIEW_LIMIT', APP_STORAGE_READ_BUFFER)instead of a hardcoded value allows deployment-specific configuration while maintaining backward compatibility with the default fallback.
1189-1189: Consistent buffer size usage.Using
APP_STORAGE_READ_BUFFERconstant standardizes the buffer size across file operations.
1204-1210: Improved header handling logic.Moving response headers after range validation ensures they're only set on valid requests. The standardized 45-day cache duration is appropriate for file downloads.
1356-1357: Consistent buffer handling across endpoints.Using
APP_STORAGE_READ_BUFFERmaintains consistency with other file endpoints.
1371-1379: Consistent header handling pattern.Response headers appropriately moved after validation, maintaining consistency with the download endpoint.
1509-1510: Consistent implementation across all file endpoints.
1524-1531: Completed consistent header handling across all endpoints.All file endpoints now follow the same pattern for response header handling and cache duration.
1007-1007: Good constant standardization for file size limit.The replacement of the hardcoded constant with
APP_STORAGE_READ_BUFFERimproves consistency across the codebase and makes the file size threshold configurable.
1204-1209: Good consolidation of response header logic.Moving the response headers after range validation and partial content status handling improves the logical flow. The cache max-age standardization to 45 days (3888000 seconds) provides consistent caching behavior.
1356-1357: Consistent range calculation improvement.This change mirrors the download endpoint improvement, maintaining consistency across file serving endpoints by using
APP_STORAGE_READ_BUFFERfor range calculations.
1371-1378: Consistent header consolidation across endpoints.The header consolidation follows the same pattern as the download endpoint, with appropriate security headers for the view endpoint (CSP and X-Content-Type-Options). The standardized 45-day cache duration is consistent.
1509-1510: Consistent range handling in push endpoint.The change maintains consistency with the download and view endpoints by using
APP_STORAGE_READ_BUFFERfor range calculations.
1524-1530: Complete header consolidation across all file endpoints.The push endpoint now follows the same header consolidation pattern as download and view endpoints, with consistent cache duration and security headers. This completes the standardization effort across all file serving endpoints.
1189-1191:Details
✅ Verification successful
Verify the range calculation logic change.
The logic has been updated to use
APP_STORAGE_READ_BUFFERinstead ofMAX_OUTPUT_CHUNK_SIZE. Ensure thatAPP_STORAGE_READ_BUFFERis appropriate for this range validation context and that it doesn't break existing chunk-based download functionality.
🏁 Script executed:
#!/bin/bash # Description: Verify that APP_STORAGE_READ_BUFFER is properly defined and used consistently # Check if APP_STORAGE_READ_BUFFER is defined rg -A 3 -B 3 "APP_STORAGE_READ_BUFFER.*=" --type php # Check other usages of this constant to ensure consistency rg "APP_STORAGE_READ_BUFFER" --type phpLength of output: 4762
APP_STORAGE_READ_BUFFER usage verified
APP_STORAGE_READ_BUFFERis defined as 20 MB inapp/init/constants.phpand is used consistently across storage and download modules to cap chunk sizes. This aligns with existing download logic—no further changes are required.tests/e2e/Services/Teams/TeamsConsoleClientTest.php (5)
68-79: Good test refactoring.Storing the response and extracting the developer user ID improves test maintainability.
94-104: Proper validation of owner deletion prevention.The test correctly validates that the last owner cannot be deleted, with appropriate error type and message assertions.
106-114: Good test cleanup practice.Removing the excess developer membership ensures consistent test state for subsequent tests.
134-134: Appropriate test adjustments for role validation.Testing with 'developer' role aligns with the new ownership protection constraints.
Also applies to: 147-147, 162-162
171-254: Comprehensive test coverage for membership deletion.Excellent test that covers all key scenarios:
- Non-existent membership (404)
- Unauthorized deletion attempts (401)
- Successful self-deletion (204)
- Owner deletion prevention (400)
The test properly validates membership counts and error messages throughout.
app/controllers/api/teams.php (4)
1103-1107: Good implementation of ownership protection.The logic correctly prevents the last owner from downgrading their role, ensuring the organization always has at least one owner. The error message is clear and helpful.
1321-1326: LGTM! Necessary dependency injections added.The addition of
userandprojectinjections is required for the new ownership validation logic in the deletion endpoint.
1334-1339: Excellent implementation of ownership protection and variable naming.The changes correctly:
- Rename the fetched user document to
$profileto avoid shadowing the injected$userparameter- Implement consistent ownership validation that prevents the last owner from leaving the organization
- Use clear variable naming with
$isSelfOwner- Provide a helpful error message
This ensures organizational continuity by preventing scenarios where an organization could be left without an owner.
Also applies to: 1350-1372, 1381-1381, 1389-1389
1106-1106:Details
✅ Verification successful
Verify that the new exception constants are properly defined.
The code uses
Exception::MEMBERSHIP_DOWNGRADE_PROHIBITEDandException::MEMBERSHIP_DELETION_PROHIBITED. Please ensure these constants are defined in the Exception class.Also applies to: 1369-1369
🏁 Script executed:
#!/bin/bash # Description: Verify that the new exception constants are defined # Check if MEMBERSHIP_DOWNGRADE_PROHIBITED is defined echo "Checking for MEMBERSHIP_DOWNGRADE_PROHIBITED constant..." rg "MEMBERSHIP_DOWNGRADE_PROHIBITED" -A 2 -B 2 # Check if MEMBERSHIP_DELETION_PROHIBITED is defined echo "Checking for MEMBERSHIP_DELETION_PROHIBITED constant..." rg "MEMBERSHIP_DELETION_PROHIBITED" -A 2 -B 2Length of output: 2977
Verified exception constants definition
Both
MEMBERSHIP_DOWNGRADE_PROHIBITEDandMEMBERSHIP_DELETION_PROHIBITEDare declared insrc/Appwrite/Extend/Exception.phpand properly mapped inapp/config/errors.php. No further changes required.
| 'deploymentId' => $deployment->isEmpty() ? '' : $deployment->getId(), | ||
| 'deploymentInternalId' => $deployment->isEmpty() ? '' : $deployment->getInternalId(), | ||
| 'deploymentResourceType' => 'site', | ||
| 'deploymentResourceId' => $site->getId(), | ||
| 'deploymentResourceInternalId' => $site->getInternalId(), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
The isEmpty() check appears redundant.
At this point in the code (line 350), the $deployment variable was just created at line 308 and shouldn't be empty. The conditional empty strings for deploymentId and deploymentInternalId may be unnecessary.
Run the following script to verify if there are any scenarios where deployment could be empty at this point:
🏁 Script executed:
#!/bin/bash
# Search for any code paths that might set deployment to empty after creation
ast-grep --pattern 'createDocument($_,$_,$deployment)'
rg -A 10 'deployment.*isEmpty\(\)' --type phpLength of output: 38172
Remove redundant isEmpty() checks in deployment response
Since we just create a new deployment when none exists, $deployment is guaranteed to be non-empty by the time we build the response. You can simplify the payload and drop the ternary fallbacks:
- In src/Appwrite/Platform/Modules/Sites/Http/Deployments/Create.php at lines 350–354, replace:
- 'deploymentId' => $deployment->isEmpty() ? '' : $deployment->getId(),
- 'deploymentInternalId' => $deployment->isEmpty() ? '' : $deployment->getInternalId(),
+ 'deploymentId' => $deployment->getId(),
+ 'deploymentInternalId' => $deployment->getInternalId(),No other changes are needed.
📝 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.
| 'deploymentId' => $deployment->isEmpty() ? '' : $deployment->getId(), | |
| 'deploymentInternalId' => $deployment->isEmpty() ? '' : $deployment->getInternalId(), | |
| 'deploymentResourceType' => 'site', | |
| 'deploymentResourceId' => $site->getId(), | |
| 'deploymentResourceInternalId' => $site->getInternalId(), | |
| 'deploymentId' => $deployment->getId(), | |
| 'deploymentInternalId' => $deployment->getInternalId(), | |
| 'deploymentResourceType' => 'site', | |
| 'deploymentResourceId' => $site->getId(), | |
| 'deploymentResourceInternalId' => $site->getInternalId(), |
🤖 Prompt for AI Agents
In src/Appwrite/Platform/Modules/Sites/Http/Deployments/Create.php around lines
350 to 354, the isEmpty() checks on the $deployment variable are redundant
because $deployment is guaranteed to be non-empty after creation at line 308.
Remove the ternary conditional checks and directly access $deployment->getId()
and $deployment->getInternalId() to simplify the response payload.
| $branchPrefix = substr($providerBranch, 0, 16); | ||
| if (strlen($providerBranch) > 16) { | ||
| $remainingChars = substr($providerBranch, 16); | ||
| $branchPrefix .= '-' . substr(hash('sha256', $remainingChars), 0, 7); | ||
| } | ||
| $resourceProjectHash = substr(hash('sha256', $resource->getId() . $project->getId()), 0, 7); | ||
| $domain = "branch-{$branchPrefix}-{$resourceProjectHash}.{$sitesDomain}"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider sanitizing branch names for domain compatibility.
While the truncation and hashing approach is good, branch names might contain characters that are invalid in domain names (e.g., underscores, spaces, special characters). Consider sanitizing the branch name before using it in the domain.
$branchPrefix = substr($providerBranch, 0, 16);
+// Sanitize branch name to ensure domain compatibility
+$branchPrefix = preg_replace('/[^a-z0-9\-]/', '-', strtolower($branchPrefix));
if (strlen($providerBranch) > 16) {
$remainingChars = substr($providerBranch, 16);
$branchPrefix .= '-' . substr(hash('sha256', $remainingChars), 0, 7);
}📝 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.
| $branchPrefix = substr($providerBranch, 0, 16); | |
| if (strlen($providerBranch) > 16) { | |
| $remainingChars = substr($providerBranch, 16); | |
| $branchPrefix .= '-' . substr(hash('sha256', $remainingChars), 0, 7); | |
| } | |
| $resourceProjectHash = substr(hash('sha256', $resource->getId() . $project->getId()), 0, 7); | |
| $domain = "branch-{$branchPrefix}-{$resourceProjectHash}.{$sitesDomain}"; | |
| $branchPrefix = substr($providerBranch, 0, 16); | |
| // Sanitize branch name to ensure domain compatibility | |
| $branchPrefix = preg_replace('/[^a-z0-9\-]/', '-', strtolower($branchPrefix)); | |
| if (strlen($providerBranch) > 16) { | |
| $remainingChars = substr($providerBranch, 16); | |
| $branchPrefix .= '-' . substr(hash('sha256', $remainingChars), 0, 7); | |
| } | |
| $resourceProjectHash = substr(hash('sha256', $resource->getId() . $project->getId()), 0, 7); | |
| $domain = "branch-{$branchPrefix}-{$resourceProjectHash}.{$sitesDomain}"; |
🤖 Prompt for AI Agents
In app/controllers/api/vcs.php around lines 296 to 302, the branch name used in
the domain may contain invalid characters for domain names. To fix this,
sanitize the $providerBranch by replacing or removing characters that are not
allowed in domain names (such as underscores, spaces, and special characters)
before truncating and hashing it. This ensures the generated domain string is
valid and avoids potential issues with domain resolution.
| Query::equal('deploymentVcsProviderBranch', ['']), | ||
| Query::equal("projectInternalId", [$project->getInternalId()]) | ||
| ]; |
There was a problem hiding this comment.
Remove duplicate projectInternalId query.
The projectInternalId query is already included at the beginning of the queries array. The duplicate at the end is redundant and may impact query performance.
Query::equal('deploymentResourceType', ['function']),
Query::equal('trigger', ['manual']),
Query::equal('deploymentVcsProviderBranch', ['']),
-Query::equal("projectInternalId", [$project->getInternalId()])Apply the same fix for the sites section at lines 1115-1117.
Also applies to: 1115-1117
🤖 Prompt for AI Agents
In src/Appwrite/Platform/Modules/Functions/Workers/Builds.php at lines 1089-1091
and 1115-1117, remove the duplicate Query::equal("projectInternalId", ...)
entries from the queries arrays. The projectInternalId query is already present
at the start of each array, so delete the redundant one at the end to avoid
unnecessary duplication and potential performance issues.
| $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.
Remove duplicate projectInternalId query (third occurrence).
Another instance of the duplicate projectInternalId query that should be removed.
Query::equal("deploymentVcsProviderBranch", [$branchName]),
Query::equal("trigger", ['manual']),
-Query::equal("projectInternalId", [$project->getInternalId()])🤖 Prompt for AI Agents
In src/Appwrite/Platform/Modules/Functions/Workers/Builds.php between lines 1172
and 1180, there is a duplicate Query::equal("projectInternalId", ...) entry
appearing twice. Remove the third occurrence of this query to eliminate
redundancy and ensure the query list contains unique conditions.
| 'key' => 'web', | ||
| 'name' => 'Web', | ||
| 'version' => '18.1.1', | ||
| 'version' => '18.1.0', |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the rationale for SDK version changes.
The SDK version updates show a mixed pattern:
- Downgrades: Web (18.1.1→18.1.0), Flutter (17.0.0→16.1.0), React Native (0.9.2→0.9.1)
- Upgrade: CLI (6.2.3→7.0.0)
Please confirm that:
- The downgrades are intentional (e.g., rolling back problematic versions)
- These versions are compatible with the current Appwrite version (1.7.4)
- No breaking changes are introduced for existing users
- The CLI upgrade to 7.0.0 doesn't break existing workflows
Also applies to: 62-62, 137-137, 220-220
🌐 Web query:
What are the latest stable versions of Appwrite SDKs for Web, Flutter, React Native, and CLI?
💡 Result:
As of June 1, 2025, the latest stable versions of the Appwrite SDKs for various platforms are:
- Web SDK: Version 16.0.1
- Flutter SDK: Version 15.0.0
- React Native SDK: Version 0.5.0 (currently in beta)
- CLI: Version 1.6.1
These versions are detailed on the official Appwrite SDKs documentation page. (appwrite.io)
For the most up-to-date information and to access the SDKs, you can visit the Appwrite SDKs documentation page. (appwrite.io)
Citations:
- 1: https://appwrite.io/docs/sdks?utm_source=openai
- 2: https://appwrite.io/docs/sdks?utm_source=openai
Update platform SDK versions to match the latest stable releases
The versions in app/config/platforms.php don’t align with the official Appwrite SDKs (as of June 1, 2025). Please correct them as follows:
• app/config/platforms.php
- Line 14 (Web SDK)
- Line 62 (Flutter SDK)
- Line 137 (React Native SDK)
- Line 220 (CLI SDK)
Proposed diff:
--- a/app/config/platforms.php
+++ b/app/config/platforms.php
@@ -14,7 +14,7 @@
- 'version' => '18.1.0',
+ 'version' => '16.0.1', // Web SDK latest stable
@@ -62,7 +62,7 @@
- 'version' => '16.1.0',
+ 'version' => '15.0.0', // Flutter SDK latest stable
@@ -137,7 +137,7 @@
- 'version' => '0.9.1',
+ 'version' => '0.5.0', // React Native SDK latest stable (beta)
@@ -220,7 +220,7 @@
- 'version' => '7.0.0',
+ 'version' => '1.6.1', // CLI latest stablePlease verify these updates against the official docs: https://appwrite.io/docs/sdks
🤖 Prompt for AI Agents
In app/config/platforms.php at lines 14, 62, 137, and 220, the SDK versions for
Web, Flutter, React Native, and CLI do not match the latest stable releases from
the official Appwrite documentation as of June 1, 2025. Update these lines to
set the Web SDK to 16.0.1, Flutter SDK to 15.0.0, React Native SDK to 0.5.0, and
CLI SDK to 1.6.1. Verify these changes against the official Appwrite SDK
documentation to ensure compatibility and avoid introducing breaking changes.
| if ($attribute->getAttribute('type') === Database::VAR_RELATIONSHIP) { | ||
| $options = $attribute->getAttribute('options'); | ||
| foreach ($options as $key => $value) { | ||
| $attribute->setAttribute($key, $value); | ||
| } | ||
| $attribute->removeAttribute('options'); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify removal of encryption handling from subQueryAttributes filter.
The subQueryAttributes filter has been simplified to only handle VAR_RELATIONSHIP attributes, removing the previous encryption-related logic for VAR_STRING attributes. This appears to be part of a broader removal of encryption functionality.
Please ensure that:
- All code that previously relied on encrypted string attributes has been updated
- Existing encrypted data can still be accessed during the transition
- Database migrations are in place if needed
🏁 Script executed:
#!/bin/bash
# Search for any remaining references to encrypted string attributes
rg -A 5 -B 5 "encrypt.*string|string.*encrypt" --type php
# Search for any code that might still expect the old filter behavior
rg -A 5 -B 5 "VAR_STRING.*encrypt|encrypt.*VAR_STRING" --type phpLength of output: 8876
Restore encrypted-attribute guard in subQueryAttributes filter
The simplification in app/init/database/filters.php removed the check that prevented querying encrypted string attributes. Since we still support the encrypt flag on VAR_STRING fields (see controllers/api/databases.php and platform collection definitions), any attempt to filter them must throw an exception.
Please update the subQueryAttributes filter (around lines 80–86) as follows:
- if ($attribute->getAttribute('type') === Database::VAR_RELATIONSHIP) {
+ // Disallow filtering encrypted string attributes
+ if ($attribute->getAttribute('type') === Database::VAR_STRING
+ && $attribute->getAttribute('encrypt') === true
+ ) {
+ throw new Exception(Exception::ATTRIBUTE_ENCRYPTED_CANNOT_BE_FILTERED);
+ }
+ if ($attribute->getAttribute('type') === Database::VAR_RELATIONSHIP) {
$options = $attribute->getAttribute('options');
foreach ($options as $key => $value) {
$attribute->setAttribute($key, $value);
}
$attribute->removeAttribute('options');
}• File: app/init/database/filters.php
• Filter: subQueryAttributes
• Lines: ~80–86
Also add/update tests to cover the encrypted-attribute filter case.
📝 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.
| if ($attribute->getAttribute('type') === Database::VAR_RELATIONSHIP) { | |
| $options = $attribute->getAttribute('options'); | |
| foreach ($options as $key => $value) { | |
| $attribute->setAttribute($key, $value); | |
| } | |
| $attribute->removeAttribute('options'); | |
| } | |
| // Disallow filtering encrypted string attributes | |
| if ($attribute->getAttribute('type') === Database::VAR_STRING | |
| && $attribute->getAttribute('encrypt') === true | |
| ) { | |
| throw new Exception(Exception::ATTRIBUTE_ENCRYPTED_CANNOT_BE_FILTERED); | |
| } | |
| if ($attribute->getAttribute('type') === Database::VAR_RELATIONSHIP) { | |
| $options = $attribute->getAttribute('options'); | |
| foreach ($options as $key => $value) { | |
| $attribute->setAttribute($key, $value); | |
| } | |
| $attribute->removeAttribute('options'); | |
| } |
🤖 Prompt for AI Agents
In app/init/database/filters.php around lines 80 to 86, the subQueryAttributes
filter currently lacks a check to prevent querying encrypted VAR_STRING
attributes, which must throw an exception to maintain encryption support. Update
the filter to reintroduce a guard that detects if a VAR_STRING attribute has the
encrypt flag set and throw an appropriate exception when such a filter is
attempted. Additionally, add or update tests to cover this encrypted-attribute
filtering case to ensure proper enforcement.
✨ 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
Summary by CodeRabbit
New Features
Bug Fixes
Chores
Tests