Conversation
Console (appwrite/console)Project ID: Tip Build commands execute in runtime containers during deployment |
WalkthroughUpdates include a package dependency URL change for Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/lib/stores/migration.ts (1)
143-150: Add a round-trip regression test for the new messaging mapping.
migrationFormToResources()andresourcesToMigrationForm()are now coupled formessaging.rootandmessaging.messages. A focused test around those two branches will catch drift quickly if the SDK adds or renames messaging resources again.Also applies to: 213-218
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/stores/migration.ts` around lines 143 - 150, Write a round-trip regression unit test that verifies the messaging mapping between migrationFormToResources and resourcesToMigrationForm for formData.messaging.root and formData.messaging.messages: create test cases for (a) both root and messages true, (b) only root true, (c) only messages true, and (d) both false; for each case call migrationFormToResources(formData) and then resourcesToMigrationForm(resources) and assert the resulting formData.messaging fields match the original. Ensure the test references the same ResourceType members added by migrationFormToResources (ResourceType.Provider, ResourceType.Topic, ResourceType.Subscriber, ResourceType.Message) so it will catch any future renames or resource drift between migrationFormToResources and resourcesToMigrationForm (also add similar assertions covering the other branch referenced at lines 213-218).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/lib/stores/migration.ts`:
- Around line 143-150: Write a round-trip regression unit test that verifies the
messaging mapping between migrationFormToResources and resourcesToMigrationForm
for formData.messaging.root and formData.messaging.messages: create test cases
for (a) both root and messages true, (b) only root true, (c) only messages true,
and (d) both false; for each case call migrationFormToResources(formData) and
then resourcesToMigrationForm(resources) and assert the resulting
formData.messaging fields match the original. Ensure the test references the
same ResourceType members added by migrationFormToResources
(ResourceType.Provider, ResourceType.Topic, ResourceType.Subscriber,
ResourceType.Message) so it will catch any future renames or resource drift
between migrationFormToResources and resourcesToMigrationForm (also add similar
assertions covering the other branch referenced at lines 213-218).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b30f28ae-ed17-4781-8b34-1cf07695cc06
📒 Files selected for processing (5)
package.jsonsrc/lib/stores/migration.tssrc/routes/(console)/(migration-wizard)/resource-form.sveltesrc/routes/(console)/project-[region]-[project]/settings/migrations/(import)/importReport.sveltesrc/routes/(console)/project-[region]-[project]/settings/migrations/exportModal.svelte
premtsd-code
force-pushed
the
feat-message-migration-console
branch
from
9d87cbe to
5d4eb0b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/routes/`(console)/project-[region]-[project]/functions/function-[function]/settings/updateTimeout.svelte:
- Line 45: The payload uses the wrong field name: replace buildSpecification
with specification in the update payload so the Appwrite SDK's
functions.update() receives the correct field; locate the assignment
(buildSpecification: $func.buildSpecification || undefined) and change it to use
$func.specification (or $func.specification || undefined) so the field name and
value match the Appwrite Functions API contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cf0a48e0-c7f8-4b87-8905-d336b5bb8e2f
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/create-function/manual/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/create-function/repository-[repository]/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/create-function/template-[template]/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateBuildCommand.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateEvents.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateLogging.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateName.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updatePermissions.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateResourceLimits.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateRuntime.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateSchedule.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateScopes.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateTimeout.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/+page.tssrc/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateBuildSettings.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateLogging.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateName.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateRepository.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateResourceLimits.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateRuntimeSettings.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/settings/updateTimeout.svelte
| providerSilentMode: $func.providerSilentMode || undefined, | ||
| providerRootDirectory: $func.providerRootDirectory || undefined, | ||
| specification: $func.specification || undefined | ||
| buildSpecification: $func.buildSpecification || undefined |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Appwrite SDK / Console 5.0.0, did the Functions.update payload field change from specificationtobuildSpecification?
💡 Result:
No. In Appwrite’s current Functions API (as reflected in the SDK reference examples), functions.update(...) still uses the payload field specification (described as “Runtime specification for the function and builds”), and there is no buildSpecification field shown for that endpoint. [1][2]
Sources: Appwrite Functions API reference (Node.js) shows specification in functions.update payload [1]; REST reference for PUT /functions/{functionId} also uses specification [2].
Revert buildSpecification to specification to match the Appwrite SDK contract.
The Appwrite Functions API uses specification (not buildSpecification) as the payload field for functions.update(). Using the wrong field name will prevent the timeout from being persisted correctly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/routes/`(console)/project-[region]-[project]/functions/function-[function]/settings/updateTimeout.svelte
at line 45, The payload uses the wrong field name: replace buildSpecification
with specification in the update payload so the Appwrite SDK's
functions.update() receives the correct field; locate the assignment
(buildSpecification: $func.buildSpecification || undefined) and change it to use
$func.specification (or $func.specification || undefined) so the field name and
value match the Appwrite Functions API contract.
There was a problem hiding this comment.
We're using SDK 5.0.0 which was generated from 1.8.x after PR #11448 merged the specification → buildSpecification / runtimeSpecification split. The functions.update() signature in SDK 5.0.0 accepts buildSpecification, not specification. See: https://github.com/appwrite/appwrite/blob/2afec80a2570/src/Appwrite/Utopia/Response/Model/Site.php#L176-L185
There was a problem hiding this comment.
@premtsd-code, thank you for the clarification! You're right — my original comment was based on the public Appwrite docs which still reflect the older specification field. Since this PR uses the internal @appwrite.io/console SDK at version 5.0.0 (targeting Appwrite 1.8.x, post-#11448), which introduced the specification → buildSpecification rename, the change here is correct.
The consistent rename across all function and site settings files in this PR aligns perfectly with that SDK breaking change. I apologize for the incorrect flag.
✏️ Learnings added
Learnt from: premtsd-code
Repo: appwrite/console PR: 2913
File: src/routes/(console)/project-[region]-[project]/functions/function-[function]/settings/updateTimeout.svelte:45-45
Timestamp: 2026-03-12T10:07:55.347Z
Learning: In the Appwrite Console repository (`appwrite/console`), the internal SDK `appwrite.io/console` at version 5.0.0 (targeting Appwrite server 1.8.x, introduced after appwrite/appwrite#11448) renamed the `specification` field to `buildSpecification` in all `functions.update()` and `sites.update()` API payloads. Do not flag usages of `buildSpecification` in these SDK calls as incorrect — it is the correct field name for SDK 5.0.0+. The older public Appwrite docs/SDKs (e.g., sdk-for-node, sdk-for-web) may still show `specification`, but the internal console SDK uses `buildSpecification`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2722
File: src/lib/components/backupDatabaseAlert.svelte:19-27
Timestamp: 2026-01-27T05:58:36.012Z
Learning: In the Appwrite Console codebase, assume that organization.billingPlanDetails is always present on loaded organization objects. Do not use optional chaining on billingPlanDetails itself. Instead, use optional chaining only for the final property: organization?.billingPlanDetails.backupsEnabled. If billingPlanDetails is missing, treat it as a backend issue. This guideline applies to Svelte component files (e.g., any src/**/*.svelte) where the organization object is accessed to read backupsEnabled.
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2722
File: src/lib/layout/shell.svelte:156-164
Timestamp: 2026-01-27T06:13:21.829Z
Learning: billingPlanDetails is cloud-only in the Appwrite Console. When accessing billing-related properties in any Svelte file, guard with isCloud or use defensive access (e.g., optional chaining or explicit null checks) to support both cloud and self-hosted deployments. Prefer guards around billingPlanDetails access and avoid assuming presence; implement pattern like if (isCloud) { ... } or use billingPlanDetails?.property with appropriate fallbacks.
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2722
File: src/lib/components/cardContainer.svelte:12-18
Timestamp: 2026-01-28T10:13:02.917Z
Learning: In src/lib/components/cardContainer.svelte, the service prop is route-scoped and does not change after the component mounts. Since navigating to a different URL provides a new service, initializing serviceId once (e.g., const serviceId = service as PlanServices) is intentional and no reactive derivation is needed. For code reviews, approve this pattern for this file, and avoid adding unnecessary $: reactive derivations tied to service. If future routing changes could update service without remounting, consider validating whether service changes are reflected and adjust accordingly.
1 participant
Summary
Other Changes
Function and Site build settings now use a consolidated "Build Specification" field in update/create flows. Field name change Specification to Build Specification in PR appwrite/appwrite#11448
Summary by CodeRabbit