Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThis pull request bumps the SDK version from 22.0.0 to 22.1.0 and introduces several significant changes: a refactored realtime subscription system with per-subscription query support and slot-based routing, removal of the Roles enum with migration to string[] type parameters in Teams, new API key and payment method management for accounts, a new Organizations service, and expanded model types including invoices and billing. Most documentation examples are reformatted with markdown code fences for consistency. OAuth providers and a Scopes enum are also added. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 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://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/account.ts (1)
492-498:⚠️ Potential issue | 🟡 MinorGuard against empty-object inputs bypassing required-parameter checks.
With the new
'key' in paramsOrFirstgates,{}now falls into the positional path and passes thetypeof ... !== 'undefined'checks, which can yield[object Object]in the URL instead of a clear missing‑parameter error. Consider short‑circuiting when a plain object lacks the expected keys (or keep the previous object detection) for these methods.💡 Example fix pattern (apply similarly to the other methods)
- if ((paramsOrFirst && typeof paramsOrFirst === 'object' && !Array.isArray(paramsOrFirst) && ('provider' in paramsOrFirst || 'success' in paramsOrFirst || 'failure' in paramsOrFirst || 'scopes' in paramsOrFirst))) { - params = (paramsOrFirst || {}) as { provider: OAuthProvider, success?: string, failure?: string, scopes?: string[] }; - } else { + if (paramsOrFirst && typeof paramsOrFirst === 'object' && !Array.isArray(paramsOrFirst)) { + if (!('provider' in paramsOrFirst || 'success' in paramsOrFirst || 'failure' in paramsOrFirst || 'scopes' in paramsOrFirst)) { + throw new AppwriteException('Missing required parameter: "provider"'); + } + params = (paramsOrFirst || {}) as { provider: OAuthProvider, success?: string, failure?: string, scopes?: string[] }; + } else { params = { provider: paramsOrFirst as OAuthProvider, success: rest[0] as string, failure: rest[1] as string, scopes: rest[2] as string[] }; }Also applies to: 544-550, 600-607, 663-670, 724-731, 776-783, 829-836, 884-891, 1897-1906, 2649-2658
🧹 Nitpick comments (2)
docs/examples/account/update-push-target.md (1)
10-13: Make the example runnable without top‑level await.Top‑level
awaitonly works in ESM contexts; for doc snippets it’s safer to wrap in an async IIFE (or add a brief note).♻️ Proposed fix
-const result = await account.updatePushTarget({ - targetId: '<TARGET_ID>', - identifier: '<IDENTIFIER>' -}); - -console.log(result); +(async () => { + const result = await account.updatePushTarget({ + targetId: '<TARGET_ID>', + identifier: '<IDENTIFIER>' + }); + + console.log(result); +})();src/services/avatars.ts (1)
53-53: Overly broad object-detection check, but safe given the required-param validation downstream.The condition now treats
paramsOrFirstas the object overload if any ofcode,width,height, orqualityis present, rather than justcode. This meansgetBrowser({ width: 200 })(missing the requiredcode) takes the object path and correctly throws"Missing required parameter: code"at Line 70, which is fine. However, sinceBrowser/CreditCard/Flagare string enums (strings at runtime), thetypeof === 'object'check alone is already sufficient to distinguish the two overloads — the additional'key' in ...OR checks are redundant.Given this appears to be generated SDK code, this is a cosmetic observation, not a blocker.
ChiragAgg5k
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/account.ts (1)
711-1016:⚠️ Potential issue | 🟠 MajorAllow
nullfor key expiration in the key APIs.The JSDoc for
createKeyandupdateKeydocuments that theexpireparameter acceptsnullfor unlimited expiration, but the TypeScript signatures only allowstring. This prevents passing valid values and misleads TypeScript users. Update method overloads to acceptstring | nulland align theModels.Key.expirefield type accordingly.Suggested signature fix
- createKey(params: { name: string, scopes: Scopes[], expire?: string }): Promise<Models.Key>; + createKey(params: { name: string, scopes: Scopes[], expire?: string | null }): Promise<Models.Key>; ... - createKey(name: string, scopes: Scopes[], expire?: string): Promise<Models.Key>; + createKey(name: string, scopes: Scopes[], expire?: string | null): Promise<Models.Key>; ... - updateKey(params: { keyId: string, name: string, scopes: Scopes[], expire?: string }): Promise<Models.Key>; + updateKey(params: { keyId: string, name: string, scopes: Scopes[], expire?: string | null }): Promise<Models.Key>; ... - updateKey(keyId: string, name: string, scopes: Scopes[], expire?: string): Promise<Models.Key>; + updateKey(keyId: string, name: string, scopes: Scopes[], expire?: string | null): Promise<Models.Key>; ... export type Key = { ... - expire: string; + expire: string | null;
🤖 Fix all issues with AI agents
In `@README.md`:
- Line 9: The README's compatibility line ("**This SDK is compatible with
Appwrite server version latest. ...**") is too vague and will age; replace
"latest" with the concrete server version number this SDK was tested against
(e.g., "1.8.x" or the exact version like "22.1.0"), or add a version
badge/variable and a short note indicating which SDK release maps to which
server version so consumers can see the tested compatibility; update that bolded
sentence accordingly and ensure the previous-releases link remains for older
versions.
In `@src/models.ts`:
- Around line 1647-1649: The JSDoc for the PaymentMethod.userId field is
incorrect ("ID of the Team"); update the comment above the userId property in
the PaymentMethod model to accurately describe the field (e.g., "User ID" or the
actual owner type if not a user). Locate the userId property in the
PaymentMethod interface/class (symbol: userId under PaymentMethod) and replace
the misleading comment with the correct short description reflecting the field's
purpose.
- Around line 1121-1123: The Key model's expire field and the account service
methods need to allow null per the documented API: change the Key
interface/property named expire from type string to string | null and update the
createKey and updateKey method signatures in src/services/account.ts so their
expire parameter is expire?: string | null (preserving optional semantics);
ensure any related type references use the updated Key.expire type so passing
null for unlimited expiration compiles.
In `@src/services/account.ts`:
- Around line 123-476: The JSDoc for listBillingAddresses incorrectly lists
non-existent filters ("expired", "failed"); update the comment above the
listBillingAddresses method to remove or correct those attributes so it only
documents valid queryable fields (e.g., "userId" or whatever the BillingAddress
model actually supports). Locate the JSDoc block for the listBillingAddresses
overloads and edit the "You may filter on the following attributes" sentence to
reflect the real BillingAddress attributes or omit the attribute list entirely.
🧹 Nitpick comments (2)
docs/examples/account/update-key.md (1)
13-14: Consider showing a realistic value or omittingexpireentirely.Passing an empty string
''for an optional date/expiry parameter is ambiguous — it's unclear whether the server interprets this as "no expiry" or rejects it. Showing a real ISO date or simply omitting the field would be more helpful to users.src/channel.ts (1)
16-16: AlignActionableChannelwith the updatedActionableunion.
ActionabledropsExecution, butActionableChannelstill includesChannel<Execution>, which can mislead consumers about which channels support create/update/delete. Consider removingExecutionor renaming the alias/comment to reflect its broader scope.♻️ Suggested tweak
-export type ActionableChannel = Channel<Document> | Channel<Row> | Channel<File> | Channel<Execution> | Channel<Team> | Channel<Membership>; +export type ActionableChannel = Channel<Document> | Channel<Row> | Channel<File> | Channel<Team> | Channel<Membership>;Also applies to: 145-145
| [](https://appwrite.io/discord) | ||
|
|
||
| **This SDK is compatible with Appwrite server version 1.8.x. For older versions, please check [previous releases](https://github.com/appwrite/sdk-for-web/releases).** | ||
| **This SDK is compatible with Appwrite server version latest. For older versions, please check [previous releases](https://github.com/appwrite/sdk-for-web/releases).** |
There was a problem hiding this comment.
Vague compatibility statement: "server version latest" will age poorly.
Replacing a specific server version (e.g., 1.8.x) with the word "latest" removes useful compatibility information. As the server evolves, users won't know which server version this SDK release was tested against. Consider keeping a concrete version number (e.g., 1.8.x or whatever version 22.1.0 targets).
🤖 Prompt for AI Agents
In `@README.md` at line 9, The README's compatibility line ("**This SDK is
compatible with Appwrite server version latest. ...**") is too vague and will
age; replace "latest" with the concrete server version number this SDK was
tested against (e.g., "1.8.x" or the exact version like "22.1.0"), or add a
version badge/variable and a short note indicating which SDK release maps to
which server version so consumers can see the tested compatibility; update that
bolded sentence accordingly and ensure the previous-releases link remains for
older versions.
| * Key expiration date in ISO 8601 format. | ||
| */ | ||
| expire: string; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the exact code in src/models.ts around lines 1121-1123
sed -n '1110,1130p' src/models.tsRepository: appwrite/sdk-for-web
Length of output: 503
🏁 Script executed:
# Also check the context around the Key model to understand the full structure
sed -n '1080,1140p' src/models.tsRepository: appwrite/sdk-for-web
Length of output: 1745
🏁 Script executed:
# Now let's look at the createKey and updateKey signatures in src/services/account.ts
grep -n "createKey\|updateKey" src/services/account.ts | head -20Repository: appwrite/sdk-for-web
Length of output: 511
🏁 Script executed:
# Search for any references to key expiration handling in the codebase
rg "expire.*null|null.*expire" --type ts -C 2Repository: appwrite/sdk-for-web
Length of output: 2126
🏁 Script executed:
# Look for test files or documentation about key expiration
find . -type f \( -name "*.test.ts" -o -name "*.spec.ts" -o -name "*.md" \) | xargs grep -l "expire\|expiration" 2>/dev/null | head -10Repository: appwrite/sdk-for-web
Length of output: 139
🏁 Script executed:
# Check the example documentation for key creation/update
cat docs/examples/account/create-key.md docs/examples/account/update-key.mdRepository: appwrite/sdk-for-web
Length of output: 924
🏁 Script executed:
# Also search for any tests or references showing null expiration values
rg "expire.*null|expire.*undefined" src/ -C 2Repository: appwrite/sdk-for-web
Length of output: 2662
Allow null for key expiration in the model and service signatures.
The JSDoc documentation in src/services/account.ts explicitly states "Use null for unlimited expiration," but the TypeScript types don't reflect this contract. The Key model defines expire: string and the createKey/updateKey method signatures use expire?: string, both of which reject null values at compile time despite the documented API contract supporting them.
Update src/models.ts to allow null:
- expire: string;
+ expire: string | null;Update the method signatures in src/services/account.ts from expire?: string to expire?: string | null for both createKey and updateKey methods.
📝 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.
| * Key expiration date in ISO 8601 format. | |
| */ | |
| expire: string; | |
| * Key expiration date in ISO 8601 format. | |
| */ | |
| expire: string | null; |
🤖 Prompt for AI Agents
In `@src/models.ts` around lines 1121 - 1123, The Key model's expire field and the
account service methods need to allow null per the documented API: change the
Key interface/property named expire from type string to string | null and update
the createKey and updateKey method signatures in src/services/account.ts so
their expire parameter is expire?: string | null (preserving optional
semantics); ensure any related type references use the updated Key.expire type
so passing null for unlimited expiration compiles.
| * ID of the Team. | ||
| */ | ||
| userId: string; |
There was a problem hiding this comment.
PaymentMethod.userId doc label is misleading.
The field is userId, but the comment says “ID of the Team.” Consider updating the comment to “User ID” (or the correct owner if it’s not a user) to avoid confusion.
📝 Suggested doc fix
- * ID of the Team.
+ * User ID.📝 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.
| * ID of the Team. | |
| */ | |
| userId: string; | |
| * User ID. | |
| */ | |
| userId: string; |
🤖 Prompt for AI Agents
In `@src/models.ts` around lines 1647 - 1649, The JSDoc for the
PaymentMethod.userId field is incorrect ("ID of the Team"); update the comment
above the userId property in the PaymentMethod model to accurately describe the
field (e.g., "User ID" or the actual owner type if not a user). Locate the
userId property in the PaymentMethod interface/class (symbol: userId under
PaymentMethod) and replace the misleading comment with the correct short
description reflecting the field's purpose.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/models.ts`:
- Around line 1439-1443: The JSDoc for BillingAddress.$id is incorrect (it says
"Region ID"); update the comment for the BillingAddress.$id property to "Billing
Address ID" (or "Address ID") to accurately describe the field in the
BillingAddress type so the property documentation matches the model.
In `@src/services/account.ts`:
- Around line 373-384: The listKeys method loses an explicit false when called
as listKeys(false) because the current guard uses !paramsOrFirst, which treats
false as "no params" and drops total; update the branching in listKeys to first
check for a boolean positional argument (typeof paramsOrFirst === 'boolean')
and, if so, set params = { total: paramsOrFirst }, otherwise handle the object
case (typeof paramsOrFirst === 'object' && !Array.isArray(paramsOrFirst)) and
default to {} when undefined; refer to the paramsOrFirst variable and
listKeys(...) signature to locate where to change the conditional so that
explicit false is preserved.
🧹 Nitpick comments (2)
src/models.ts (2)
1460-1471: Inconsistent JSDoc casing forcity,state, andpostalCode.These comments use lowercase ("city", "state", "postal code") while the rest of the model and codebase capitalize the first letter of doc comments. Minor nit for consistency.
Proposed fix
/** - * city + * City. */ city: string; /** - * state + * State. */ state: string; /** - * postal code + * Postal code. */ postalCode: string;
1614-1616: Type-level JSDoc says "paymentMethod" (lowercase) — should be "PaymentMethod".Line 1615's doc comment reads
paymentMethodwhich is inconsistent with all other model doc comments that use PascalCase.Proposed fix
/** - * paymentMethod + * PaymentMethod */
src/models.ts
Outdated
| export type BillingAddress = { | ||
| /** | ||
| * Region ID | ||
| */ | ||
| $id: string; |
There was a problem hiding this comment.
BillingAddress.$id JSDoc says "Region ID" — likely a copy-paste error.
The comment should read "Billing Address ID" (or simply "Address ID") to match the model's purpose.
Proposed fix
- /**
- * Region ID
- */
+ /**
+ * Billing Address ID.
+ */
$id: string;🤖 Prompt for AI Agents
In `@src/models.ts` around lines 1439 - 1443, The JSDoc for BillingAddress.$id is
incorrect (it says "Region ID"); update the comment for the BillingAddress.$id
property to "Billing Address ID" (or "Address ID") to accurately describe the
field in the BillingAddress type so the property documentation matches the
model.
| listKeys( | ||
| paramsOrFirst?: { total?: boolean } | boolean | ||
| ): Promise<Models.KeyList> { | ||
| let params: { total?: boolean }; | ||
|
|
||
| if (!paramsOrFirst || (paramsOrFirst && typeof paramsOrFirst === 'object' && !Array.isArray(paramsOrFirst))) { | ||
| params = (paramsOrFirst || {}) as { total?: boolean }; | ||
| } else { | ||
| params = { | ||
| total: paramsOrFirst as boolean | ||
| }; | ||
| } |
There was a problem hiding this comment.
listKeys(false) silently drops the total parameter.
When a user calls listKeys(false) (the deprecated positional style), the !paramsOrFirst guard evaluates !false → true, routing into the object branch where params becomes {} and total is lost. The user's intent to disable total counting is silently ignored.
This is specific to listKeys because it's the only method whose sole positional parameter is an optional boolean.
Proposed fix
listKeys(
paramsOrFirst?: { total?: boolean } | boolean
): Promise<Models.KeyList> {
let params: { total?: boolean };
- if (!paramsOrFirst || (paramsOrFirst && typeof paramsOrFirst === 'object' && !Array.isArray(paramsOrFirst))) {
+ if (paramsOrFirst === undefined || paramsOrFirst === null || (typeof paramsOrFirst === 'object' && !Array.isArray(paramsOrFirst))) {
params = (paramsOrFirst || {}) as { total?: boolean };
} else {
params = {
total: paramsOrFirst as boolean
};
}🤖 Prompt for AI Agents
In `@src/services/account.ts` around lines 373 - 384, The listKeys method loses an
explicit false when called as listKeys(false) because the current guard uses
!paramsOrFirst, which treats false as "no params" and drops total; update the
branching in listKeys to first check for a boolean positional argument (typeof
paramsOrFirst === 'boolean') and, if so, set params = { total: paramsOrFirst },
otherwise handle the object case (typeof paramsOrFirst === 'object' &&
!Array.isArray(paramsOrFirst)) and default to {} when undefined; refer to the
paramsOrFirst variable and listKeys(...) signature to locate where to change the
conditional so that explicit false is preserved.
3 participants
This PR contains updates to the Web SDK for version 22.1.0.