Conversation
WalkthroughThis pull request updates the Appwrite Dart SDK to version 21.1.0 with multiple enhancements. Documentation examples across account, avatars, databases, functions, graphql, locale, messaging, storage, tablesdb, and teams services are standardized with proper Dart code fence formatting and client initialization examples. The Teams service is refactored to accept Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@lib/src/realtime_mixin.dart`:
- Around line 96-113: The current connected-event handler
(RealtimeResponseConnected.fromMap) unconditionally clears _slotToSubscriptionId
and _subscriptionIdToSlot which can drop valid mappings; instead, stop calling
clear(), merge the incoming subscriptions into the existing maps by parsing each
slotStr with int.tryParse and updating _slotToSubscriptionId[slot] and
_subscriptionIdToSlot[subscriptionId.toString()], and only remove mappings if
you detect an explicit server-side teardown for that slot; additionally, detect
any expected slots missing from the server response and emit a warning log
mentioning the missing slot(s) so operators can debug transient reconnect
issues.
- Around line 265-307: subscribeTo currently schedules _createSocket but never
resets the _reconnect flag after the last subscription closed, so a subsequent
socket failure won't trigger retries; ensure _reconnect is set to true whenever
you intend to actively (re)open a socket: set _reconnect = true immediately
before scheduling Future.delayed(..., () => _createSocket()) in subscribeTo (and
likewise set _reconnect = true right before calling _createSocket() inside the
subscription close callback when remainingChannels.isNotEmpty) so that _retry()
sees _reconnect == true on first failure and will perform retries; reference the
subscribeTo method, the close closure inside RealtimeSubscription, and the
_createSocket/_retry/_reconnect symbols when making the change.
🧹 Nitpick comments (8)
docs/examples/databases/increment-document-attribute.md (1)
1-1: LGTM! Proper Markdown formatting applied.The addition of Markdown code fences correctly formats the Dart example for proper syntax highlighting and readability.
Minor pre-existing documentation quality note: Line 14 uses an empty string for the
attributeparameter (attribute: ''). While not introduced by this PR, consider using a placeholder like'<ATTRIBUTE_NAME>'to better guide users.Also applies to: 19-19
docs/examples/teams/create-membership.md (1)
10-18: Consider showing a non-empty example for therolesparameter.Since
rolesis a required parameter, an empty list isn't a very instructive example. Showing a realistic value likeroles: ['owner']would better demonstrate the new string-based roles API to developers.📝 Suggested example improvement
Membership result = await teams.createMembership( teamId: '<TEAM_ID>', - roles: [], + roles: ['owner'], email: 'email@example.com', // optionaldocs/examples/teams/update-membership.md (1)
10-14: Documentation example passes an emptyroleslist — consider showing a realistic value.Since roles are now plain strings (after removing the
Rolesenum), this example could better illustrate the new API by showing a non-empty list, e.g.roles: ['admin'], so users know the expected format.📝 Suggested improvement
Membership result = await teams.updateMembership( teamId: '<TEAM_ID>', membershipId: '<MEMBERSHIP_ID>', - roles: [], + roles: ['<ROLE>'], );test/services/teams_test.dart (1)
185-189: Tests only exercise empty roles — no coverage for actual string role values.Both
createMembershipandupdateMembershiptests passroles: []. Since the signature changed fromList<enums.Roles>toList<String>, it would be valuable to test with a non-empty list (e.g.,['admin', 'member']) and verify the payload sent to the client contains the expected strings.test/src/realtime_subscription_test.dart (1)
13-14: Consider adding an assertion for the newqueriesfield.The test now passes
queries: const []but doesn't verifysubscription.queries. For consistency with the existing assertions, add a check.Suggested addition
expect(subscription.close, equals(mockCloseFunction)); + expect(subscription.queries, equals(const []));lib/src/realtime_mixin.dart (3)
229-250: Hardcoded JSON query string is fragile.The
selectAllQuerystring on line 231 duplicates the serialization format ofQuery.select(["*"]). If theQueryclass serialization format ever changes, this will silently produce incorrect requests.Suggested alternatives
Option A — Extract to a well-documented constant so it's easy to find and update:
+// Mirrors Query.select(["*"]). Must be kept in sync with the Query class. +const _kSelectAllQuery = '{"method":"select","values":["*"]}';Option B — If
Querycan be made accessible to this mixin (e.g., by importing it), use it directly to avoid drift.
283-303: Close callback doesn't awaitcontroller.close().
controller.close()on line 290 returns aFuturethat completes when all stream listeners have been notified. Not awaiting it means the downstream cleanup (socket rebuild or connection close) proceeds before listeners are guaranteed to have received the done event. In practice this is often acceptable for broadcast controllers, but if consumers rely onawait subscription.close()to guarantee stream completion, this could be surprising.Proposed fix
- controller.close(); + await controller.close();
252-256: Port handling: stripping port 80/443 regardless of scheme.The condition at lines 252-254 omits the port for both 80 and 443 unconditionally. An unconventional setup running
ws://host:443/...orwss://host:80/...would have its port silently dropped. This is an unlikely edge case, but worth noting.
| // channels, user, subscriptions? | ||
| final message = RealtimeResponseConnected.fromMap(data.data); | ||
|
|
||
| // Store subscription ID mappings from backend | ||
| // Format: { "0": "sub_a1f9", "1": "sub_b83c", ... } | ||
| _slotToSubscriptionId.clear(); | ||
| _subscriptionIdToSlot.clear(); | ||
| if (data.data['subscriptions'] != null) { | ||
| final subscriptions = | ||
| data.data['subscriptions'] as Map<String, dynamic>; | ||
| subscriptions.forEach((slotStr, subscriptionId) { | ||
| final slot = int.tryParse(slotStr); | ||
| if (slot != null) { | ||
| _slotToSubscriptionId[slot] = subscriptionId.toString(); | ||
| _subscriptionIdToSlot[subscriptionId.toString()] = slot; | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Clearing subscription ID maps on every connected event could drop mappings during transient reconnects.
_slotToSubscriptionId.clear() and _subscriptionIdToSlot.clear() at lines 101-102 discard all mappings before repopulating. If the server sends a connected message that omits a slot (e.g., due to a transient server-side issue), that slot will permanently lose its routing entry and never receive events until the next full reconnect.
Consider logging a warning when an expected slot is missing from the server's response, to aid debugging.
🤖 Prompt for AI Agents
In `@lib/src/realtime_mixin.dart` around lines 96 - 113, The current
connected-event handler (RealtimeResponseConnected.fromMap) unconditionally
clears _slotToSubscriptionId and _subscriptionIdToSlot which can drop valid
mappings; instead, stop calling clear(), merge the incoming subscriptions into
the existing maps by parsing each slotStr with int.tryParse and updating
_slotToSubscriptionId[slot] and
_subscriptionIdToSlot[subscriptionId.toString()], and only remove mappings if
you detect an explicit server-side teardown for that slot; additionally, detect
any expected slots missing from the server response and emit a warning log
mentioning the missing slot(s) so operators can debug transient reconnect
issues.
| RealtimeSubscription subscribeTo(List<Object> channels, | ||
| [List<String> queries = const []]) { | ||
| StreamController<RealtimeMessage> controller = StreamController.broadcast(); | ||
| final channelStrings = | ||
| channels.map((ch) => _channelToString(ch)).toList().cast<String>(); | ||
| _channels.addAll(channelStrings); | ||
| Future.delayed(Duration.zero, () => _createSocket()); | ||
| int id = DateTime.now().microsecondsSinceEpoch; | ||
| final queryStrings = List<String>.from(queries); | ||
|
|
||
| // Allocate a new slot index | ||
| _subscriptionsCounter++; | ||
| final slot = _subscriptionsCounter; | ||
|
|
||
| // Store slot-centric data: channels, queries, and callback belong to the slot | ||
| // queries is stored as List<String> (array of query strings) | ||
| // No channel mutation occurs here - channels are derived from slots in _createSocket() | ||
| RealtimeSubscription subscription = RealtimeSubscription( | ||
| controller: controller, | ||
| channels: channelStrings, | ||
| queries: queryStrings, | ||
| close: () async { | ||
| _subscriptions.remove(id); | ||
| final subscriptionId = _slotToSubscriptionId[slot]; | ||
| _subscriptions.remove(slot); | ||
| _slotToSubscriptionId.remove(slot); | ||
| if (subscriptionId != null) { | ||
| _subscriptionIdToSlot.remove(subscriptionId); | ||
| } | ||
| controller.close(); | ||
| _cleanup(channelStrings); | ||
|
|
||
| if (_channels.isNotEmpty) { | ||
| // Rebuild channels from remaining slots | ||
| final remainingChannels = <String>{}; | ||
| for (var sub in _subscriptions.values) { | ||
| remainingChannels.addAll(sub.channels); | ||
| } | ||
|
|
||
| if (remainingChannels.isNotEmpty) { | ||
| await Future.delayed(Duration.zero, () => _createSocket()); | ||
| } else { | ||
| await _closeConnection(); | ||
| } | ||
| }); | ||
| _subscriptions[id] = subscription; | ||
| _subscriptions[slot] = subscription; | ||
|
|
||
| Future.delayed(Duration.zero, () => _createSocket()); | ||
| return subscription; |
There was a problem hiding this comment.
After all subscriptions close, _reconnect remains false — next subscribeTo may miss its first retry.
When the last subscription closes, _closeConnection() (line 301) sets _reconnect = false. A subsequent subscribeTo call schedules _createSocket but never resets _reconnect. If that socket creation fails, _retry() sees _reconnect == false, skips the retry, and only sets _reconnect = true for the next failure.
Proposed fix
RealtimeSubscription subscribeTo(List<Object> channels,
[List<String> queries = const []]) {
+ _reconnect = true;
StreamController<RealtimeMessage> controller = StreamController.broadcast();📝 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.
| RealtimeSubscription subscribeTo(List<Object> channels, | |
| [List<String> queries = const []]) { | |
| StreamController<RealtimeMessage> controller = StreamController.broadcast(); | |
| final channelStrings = | |
| channels.map((ch) => _channelToString(ch)).toList().cast<String>(); | |
| _channels.addAll(channelStrings); | |
| Future.delayed(Duration.zero, () => _createSocket()); | |
| int id = DateTime.now().microsecondsSinceEpoch; | |
| final queryStrings = List<String>.from(queries); | |
| // Allocate a new slot index | |
| _subscriptionsCounter++; | |
| final slot = _subscriptionsCounter; | |
| // Store slot-centric data: channels, queries, and callback belong to the slot | |
| // queries is stored as List<String> (array of query strings) | |
| // No channel mutation occurs here - channels are derived from slots in _createSocket() | |
| RealtimeSubscription subscription = RealtimeSubscription( | |
| controller: controller, | |
| channels: channelStrings, | |
| queries: queryStrings, | |
| close: () async { | |
| _subscriptions.remove(id); | |
| final subscriptionId = _slotToSubscriptionId[slot]; | |
| _subscriptions.remove(slot); | |
| _slotToSubscriptionId.remove(slot); | |
| if (subscriptionId != null) { | |
| _subscriptionIdToSlot.remove(subscriptionId); | |
| } | |
| controller.close(); | |
| _cleanup(channelStrings); | |
| if (_channels.isNotEmpty) { | |
| // Rebuild channels from remaining slots | |
| final remainingChannels = <String>{}; | |
| for (var sub in _subscriptions.values) { | |
| remainingChannels.addAll(sub.channels); | |
| } | |
| if (remainingChannels.isNotEmpty) { | |
| await Future.delayed(Duration.zero, () => _createSocket()); | |
| } else { | |
| await _closeConnection(); | |
| } | |
| }); | |
| _subscriptions[id] = subscription; | |
| _subscriptions[slot] = subscription; | |
| Future.delayed(Duration.zero, () => _createSocket()); | |
| return subscription; | |
| RealtimeSubscription subscribeTo(List<Object> channels, | |
| [List<String> queries = const []]) { | |
| _reconnect = true; | |
| StreamController<RealtimeMessage> controller = StreamController.broadcast(); | |
| final channelStrings = | |
| channels.map((ch) => _channelToString(ch)).toList().cast<String>(); | |
| final queryStrings = List<String>.from(queries); | |
| // Allocate a new slot index | |
| _subscriptionsCounter++; | |
| final slot = _subscriptionsCounter; | |
| // Store slot-centric data: channels, queries, and callback belong to the slot | |
| // queries is stored as List<String> (array of query strings) | |
| // No channel mutation occurs here - channels are derived from slots in _createSocket() | |
| RealtimeSubscription subscription = RealtimeSubscription( | |
| controller: controller, | |
| channels: channelStrings, | |
| queries: queryStrings, | |
| close: () async { | |
| final subscriptionId = _slotToSubscriptionId[slot]; | |
| _subscriptions.remove(slot); | |
| _slotToSubscriptionId.remove(slot); | |
| if (subscriptionId != null) { | |
| _subscriptionIdToSlot.remove(subscriptionId); | |
| } | |
| controller.close(); | |
| // Rebuild channels from remaining slots | |
| final remainingChannels = <String>{}; | |
| for (var sub in _subscriptions.values) { | |
| remainingChannels.addAll(sub.channels); | |
| } | |
| if (remainingChannels.isNotEmpty) { | |
| await Future.delayed(Duration.zero, () => _createSocket()); | |
| } else { | |
| await _closeConnection(); | |
| } | |
| }); | |
| _subscriptions[slot] = subscription; | |
| Future.delayed(Duration.zero, () => _createSocket()); | |
| return subscription; |
🤖 Prompt for AI Agents
In `@lib/src/realtime_mixin.dart` around lines 265 - 307, subscribeTo currently
schedules _createSocket but never resets the _reconnect flag after the last
subscription closed, so a subsequent socket failure won't trigger retries;
ensure _reconnect is set to true whenever you intend to actively (re)open a
socket: set _reconnect = true immediately before scheduling Future.delayed(...,
() => _createSocket()) in subscribeTo (and likewise set _reconnect = true right
before calling _createSocket() inside the subscription close callback when
remainingChannels.isNotEmpty) so that _retry() sees _reconnect == true on first
failure and will perform retries; reference the subscribeTo method, the close
closure inside RealtimeSubscription, and the _createSocket/_retry/_reconnect
symbols when making the change.
ChiragAgg5k
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@CHANGELOG.md`:
- Around line 3-7: Reword the changelog line about the Teams service to clarify
this is a breaking change: replace "Fix `Roles` enum removed from Teams service;
`roles` parameter now accepts `List<String>`" with wording that states the
`Roles` enum was removed and the `roles` parameter now accepts `List<String>` as
a breaking change (e.g., "Breaking change: `Roles` enum removed from Teams
service; `roles` parameter now accepts `List<String>` — update callers to pass a
list of strings"). Mention `Roles` and `roles` explicitly so readers can find
the affected API.
| ## 21.1.0 | ||
|
|
||
| * Add `queries` parameter to Realtime subscriptions for filtering events | ||
| * Fix `Roles` enum removed from Teams service; `roles` parameter now accepts `List<String>` | ||
| * Fix doc examples with proper formatting and syntax highlighting |
There was a problem hiding this comment.
Changelog entry wording on Line 6 is slightly misleading.
"Fix Roles enum removed" reads as if the removal itself is a bug fix, but removing an enum and changing the parameter type to List<String> is a breaking change. Consider rewording to clarify intent, e.g.:
Suggested wording
-* Fix `Roles` enum removed from Teams service; `roles` parameter now accepts `List<String>`
+* **Breaking**: Removed `Roles` enum from Teams service; `roles` parameter now accepts `List<String>`📝 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.
| ## 21.1.0 | |
| * Add `queries` parameter to Realtime subscriptions for filtering events | |
| * Fix `Roles` enum removed from Teams service; `roles` parameter now accepts `List<String>` | |
| * Fix doc examples with proper formatting and syntax highlighting | |
| ## 21.1.0 | |
| * Add `queries` parameter to Realtime subscriptions for filtering events | |
| * **Breaking**: Removed `Roles` enum from Teams service; `roles` parameter now accepts `List<String>` | |
| * Fix doc examples with proper formatting and syntax highlighting |
🤖 Prompt for AI Agents
In `@CHANGELOG.md` around lines 3 - 7, Reword the changelog line about the Teams
service to clarify this is a breaking change: replace "Fix `Roles` enum removed
from Teams service; `roles` parameter now accepts `List<String>`" with wording
that states the `Roles` enum was removed and the `roles` parameter now accepts
`List<String>` as a breaking change (e.g., "Breaking change: `Roles` enum
removed from Teams service; `roles` parameter now accepts `List<String>` —
update callers to pass a list of strings"). Mention `Roles` and `roles`
explicitly so readers can find the affected API.
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@docs/examples/account/create-key.md`:
- Line 14: The example sets expire: '' which is misleading; update the example
to either remove the expire property entirely (since it's optional) or replace
the empty string with a realistic ISO timestamp placeholder like
'2025-12-31T00:00:00.000+00:00' so the optional parameter expire is shown
correctly and clearly; change the expire line in the create-key example
accordingly.
In `@lib/channel.dart`:
- Around line 70-71: The comment listing actionable marker classes incorrectly
includes _Execution; edit the comment in lib/channel.dart (around the actionable
types comment) to remove `_Execution` so it only lists the real actionable
marker classes: `_Document`, `_Row`, `_File`, `_Team`, and `_Membership`; leave
the rest of the comment text intact so it accurately reflects that `_Execution`
has no create()/update()/delete() extension and is triggered via
Functions.createExecution().
In `@lib/services/account.dart`:
- Line 99: Replace the incorrect contraction "it's" with the possessive "its" in
the doc comments found in lib/services/account.dart—specifically update the
comment "Get a specific billing address for a user using it's ID." and the other
occurrences of "it's" (e.g., the comment containing "including it's scopes") so
they read "its"; search for doc comments containing the exact substring "it's"
and correct them (e.g., comments above the billing-address retrieval/update
functions and the comment about scopes).
- Around line 887-891: In updatePaymentMethodProvider the apiParams map always
includes 'state' even when the String? state is null; change construction of
apiParams so 'state' is only added when state != null (same pattern used in
updatePaymentMethod), i.e. build apiParams with providerMethodId and name, and
conditionally add 'state': state when non-null to avoid sending null to the API.
- Around line 270-274: In createKey and updateKey, apiParams currently always
include the 'expire' entry which can be null; update both functions (createKey,
updateKey) to only add 'expire' to the apiParams map when expire != null (mirror
the pattern used in createBillingAddress for addressLine2), i.e., build
apiParams without 'expire' and then conditionally insert apiParams['expire'] =
expire only when expire is not null so the API won't receive "expire": null.
In `@lib/services/organizations.dart`:
- Line 24: Fix the grammatical typo in the documentation comments: replace the
contraction "it's" with the possessive "its" in the comment above the
billing-address getter and the other occurrence around line 91; locate the doc
comments associated with the billing address method (e.g., the comment for
getBillingAddress or in the Organizations service class) and update both
instances to read "its" instead of "it's".
In `@lib/src/models/billing_address.dart`:
- Around line 5-6: The doc comment for the billing address model's field final
String $id is incorrect ("Region ID"); update the comment above the $id field in
billing_address.dart to accurately describe it (e.g., "Billing address ID" or
"Billing address unique identifier") so it reflects the field's purpose; ensure
the comment sits immediately above the declaration of $id and follows the
project's doc comment style.
In `@lib/src/models/estimation_delete_organization.dart`:
- Around line 12-17: EstimationDeleteOrganization.fromMap currently calls
map['unpaidInvoices'].map(...) which will throw if the key is missing or null;
update the factory to defensively handle missing/null by treating
map['unpaidInvoices'] as a list with a fallback (e.g., (map['unpaidInvoices'] ??
[]) ), then safely cast/iterate and map each item via Invoice.fromMap to produce
unpaidInvoices; ensure you use the same pattern as Key.fromMap (default to []
and then List<Invoice>.from(...)) and guard against non-iterable values before
mapping.
In `@lib/src/models/payment_method_list.dart`:
- Around line 16-22: PaymentMethodList.fromMap currently calls
map['paymentMethods'].map(...) which will throw if the key is missing or null;
update PaymentMethodList.fromMap to defensively handle a missing/null
paymentMethods by using map['paymentMethods'] ?? [] and then converting each
entry with PaymentMethod.fromMap, ensuring the resulting paymentMethods is a
List<PaymentMethod> (same pattern used in Key.fromMap) so parsing remains
null-safe.
In `@lib/src/models/payment_method.dart`:
- Around line 26-28: The doc comment for the field userId is incorrect ("ID of
the Team"); update the comment above the declaration of final String userId to
accurately describe the field (e.g., "ID of the user" or "ID of the user who
owns this payment method") so the documentation matches the field name (userId)
in the PaymentMethod model.
🧹 Nitpick comments (9)
docs/examples/account/update-key.md (1)
15-15: Consider using a meaningful placeholder for the optionalexpireparameter.An empty string
''is a confusing example value. Either omit it entirely (since it's optional) or use a placeholder like'2025-12-31T23:59:59.000+00:00'to indicate the expected format.lib/src/models/payment_method.dart (1)
96-109: No type coercion forintandboolfields infromMap.
expiryMonth,expiryYearare assigned directly withoutas intor a safe cast, andxdefault,expired,failedsimilarly lackas bool. If the map values arrive as unexpected types (e.g.,Stringfrom some JSON decoders), this will throw at runtime. Other string fields in this model use.toString()defensively — consider applying similar guards here for consistency.That said, if this is a generated SDK and this pattern is used consistently for
int/boolacross the codebase, this may be intentional.lib/services/organizations.dart (1)
8-22:deletereturns untypedFuture— considerFuture<void>orFuture<dynamic>for clarity.The other methods in this class specify their return type explicitly. The
deletemethod returnsres.datawith a bareFuture, which leaves the API contract ambiguous for consumers.test/services/organizations_test.dart (2)
60-70: Thedelete()test has no assertion.Unlike the other tests in this file, the
deletetest doesn't assert anything about the response or verify the invocation. Consider adding at least averifycall to confirmclient.callwas invoked with the expected HTTP method and path.
63-65: Mock stubs only match onHttpMethod, not on path or parameters.All
when(client.call(...))stubs match solely on the HTTP method, ignoringpath,params, andheaders. This means any call with the matching HTTP method will return the stubbed response, regardless of the actual endpoint invoked. While this may be the established pattern for auto-generated SDK tests, it provides weak verification that the service methods are calling the correct API endpoints.Also applies to: 84-86, 100-102, 126-128, 164-166
lib/channel.dart (1)
96-104: Inconsistent default handling between intermediate and leaf channel methods.
collection()andtable()default toid ?? '*'(always adding an ID segment), whiledocument(),row(), andfile()passidas-is (omitting the segment whennull). This is presumably intentional — intermediate segments always need an identifier for path correctness, while leaf segments omit it to mean "all" — but there's no doc comment explaining this design choice. A brief comment on the convention would help future maintainers.Also applies to: 110-116, 122-124
test/src/models/estimation_delete_organization_test.dart (1)
7-14: Consider adding a test case with a non-emptyunpaidInvoiceslist.Testing only with an empty list doesn't exercise the nested
Invoice.fromMap/Invoice.toMapserialization path. A case with at least oneInvoiceelement would catch mapping bugs in the nested model.lib/src/models/billing_address.dart (2)
40-51:fromMapsilently convertsnullvalues to the string"null".All fields use
.toString()on potentially null map values, which convertsnullto the literal string"null"rather than throwing or providing a meaningful default. This is consistent with other Appwrite SDK models (auto-generated), so flagging just for awareness — if the API ever omits a field, you'll get"null"strings silently propagated.
20-27: Inconsistent doc comment capitalization.Lines 20, 23, 26 use lowercase ("city", "state", "postal code") while Lines 5, 8, 11, 14, 17 use title/sentence case. Minor nit for consistency.
| Key result = await account.createKey( | ||
| name: '<NAME>', | ||
| scopes: [enums.Scopes.account], | ||
| expire: '', // optional |
There was a problem hiding this comment.
Empty string for optional expire parameter is misleading.
Passing expire: '' suggests a valid value, but an empty string likely isn't meaningful for an expiration date. Consider either omitting the parameter entirely (since it's optional) or showing a placeholder like '2025-12-31T00:00:00.000+00:00'.
🤖 Prompt for AI Agents
In `@docs/examples/account/create-key.md` at line 14, The example sets expire: ''
which is misleading; update the example to either remove the expire property
entirely (since it's optional) or replace the empty string with a realistic ISO
timestamp placeholder like '2025-12-31T00:00:00.000+00:00' so the optional
parameter expire is shown correctly and clearly; change the expire line in the
create-key example accordingly.
lib/services/account.dart
Outdated
| return models.BillingAddress.fromMap(res.data); | ||
| } | ||
|
|
||
| /// Get a specific billing address for a user using it's ID. |
There was a problem hiding this comment.
Minor typo: "it's" → "its" (possessive) in doc comments.
Lines 99, 115, and 146 use "it's" (contraction of "it is") where "its" (possessive) is intended. E.g., "using it's ID" → "using its ID". Also present at Line 287 ("including it's scopes").
Also applies to: 115-115, 146-146
🤖 Prompt for AI Agents
In `@lib/services/account.dart` at line 99, Replace the incorrect contraction
"it's" with the possessive "its" in the doc comments found in
lib/services/account.dart—specifically update the comment "Get a specific
billing address for a user using it's ID." and the other occurrences of "it's"
(e.g., the comment containing "including it's scopes") so they read "its";
search for doc comments containing the exact substring "it's" and correct them
(e.g., comments above the billing-address retrieval/update functions and the
comment about scopes).
| final Map<String, dynamic> apiParams = { | ||
| 'name': name, | ||
| 'scopes': scopes.map((e) => e.value).toList(), | ||
| 'expire': expire, | ||
| }; |
There was a problem hiding this comment.
expire is sent even when null, unlike other optional parameters.
In createKey, the expire parameter is optional (String?) but is always included in apiParams without a null guard. Compare with createBillingAddress (Line 84-86) which uses if (addressLine2 != null). This will send "expire": null to the API, which may cause unexpected behavior.
The same issue exists in updateKey (Line 313).
Proposed fix
final Map<String, dynamic> apiParams = {
'name': name,
'scopes': scopes.map((e) => e.value).toList(),
- 'expire': expire,
+ if (expire != null) 'expire': expire,
};Apply the same fix in updateKey at Line 313.
📝 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.
| final Map<String, dynamic> apiParams = { | |
| 'name': name, | |
| 'scopes': scopes.map((e) => e.value).toList(), | |
| 'expire': expire, | |
| }; | |
| final Map<String, dynamic> apiParams = { | |
| 'name': name, | |
| 'scopes': scopes.map((e) => e.value).toList(), | |
| if (expire != null) 'expire': expire, | |
| }; |
🤖 Prompt for AI Agents
In `@lib/services/account.dart` around lines 270 - 274, In createKey and
updateKey, apiParams currently always include the 'expire' entry which can be
null; update both functions (createKey, updateKey) to only add 'expire' to the
apiParams map when expire != null (mirror the pattern used in
createBillingAddress for addressLine2), i.e., build apiParams without 'expire'
and then conditionally insert apiParams['expire'] = expire only when expire is
not null so the API won't receive "expire": null.
| final Map<String, dynamic> apiParams = { | ||
| 'providerMethodId': providerMethodId, | ||
| 'name': name, | ||
| 'state': state, | ||
| }; |
There was a problem hiding this comment.
state is sent even when null in updatePaymentMethodProvider.
The state parameter is optional (String?) but is unconditionally included in apiParams. Compare with updatePaymentMethod (Line 848) which correctly uses if (state != null) 'state': state.
Proposed fix
final Map<String, dynamic> apiParams = {
'providerMethodId': providerMethodId,
'name': name,
- 'state': state,
+ if (state != null) 'state': state,
};📝 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.
| final Map<String, dynamic> apiParams = { | |
| 'providerMethodId': providerMethodId, | |
| 'name': name, | |
| 'state': state, | |
| }; | |
| final Map<String, dynamic> apiParams = { | |
| 'providerMethodId': providerMethodId, | |
| 'name': name, | |
| if (state != null) 'state': state, | |
| }; |
🤖 Prompt for AI Agents
In `@lib/services/account.dart` around lines 887 - 891, In
updatePaymentMethodProvider the apiParams map always includes 'state' even when
the String? state is null; change construction of apiParams so 'state' is only
added when state != null (same pattern used in updatePaymentMethod), i.e. build
apiParams with providerMethodId and name, and conditionally add 'state': state
when non-null to avoid sending null to the API.
lib/services/organizations.dart
Outdated
| return res.data; | ||
| } | ||
|
|
||
| /// Get a billing address using it's ID. |
There was a problem hiding this comment.
Minor grammar: "it's" → "its" (possessive, not contraction).
Same issue on Line 91.
- /// Get a billing address using it's ID.
+ /// Get a billing address using its ID.- /// Get an organization's payment method using it's payment method ID.
+ /// Get an organization's payment method using its payment method ID.🤖 Prompt for AI Agents
In `@lib/services/organizations.dart` at line 24, Fix the grammatical typo in the
documentation comments: replace the contraction "it's" with the possessive "its"
in the comment above the billing-address getter and the other occurrence around
line 91; locate the doc comments associated with the billing address method
(e.g., the comment for getBillingAddress or in the Organizations service class)
and update both instances to read "its" instead of "it's".
lib/src/models/billing_address.dart
Outdated
| /// Region ID | ||
| final String $id; |
There was a problem hiding this comment.
Doc comment says "Region ID" — should this be "Billing address ID"?
The $id field's doc comment says "Region ID" which seems like a copy-paste error for a billing address model.
Proposed fix
- /// Region ID
+ /// Billing address ID
final String $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.
| /// Region ID | |
| final String $id; | |
| /// Billing address ID | |
| final String $id; |
🤖 Prompt for AI Agents
In `@lib/src/models/billing_address.dart` around lines 5 - 6, The doc comment for
the billing address model's field final String $id is incorrect ("Region ID");
update the comment above the $id field in billing_address.dart to accurately
describe it (e.g., "Billing address ID" or "Billing address unique identifier")
so it reflects the field's purpose; ensure the comment sits immediately above
the declaration of $id and follows the project's doc comment style.
| factory EstimationDeleteOrganization.fromMap(Map<String, dynamic> map) { | ||
| return EstimationDeleteOrganization( | ||
| unpaidInvoices: List<Invoice>.from( | ||
| map['unpaidInvoices'].map((p) => Invoice.fromMap(p))), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Null-safety: map['unpaidInvoices'] will throw if the key is missing or null.
Other models in this PR (e.g., Key.fromMap) defensively default lists with ?? []. Here, if the server omits unpaidInvoices, .map() on null will throw a NoSuchMethodError.
Proposed fix
- unpaidInvoices: List<Invoice>.from(
- map['unpaidInvoices'].map((p) => Invoice.fromMap(p))),
+ unpaidInvoices: List<Invoice>.from(
+ (map['unpaidInvoices'] ?? []).map((p) => Invoice.fromMap(p))),📝 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.
| factory EstimationDeleteOrganization.fromMap(Map<String, dynamic> map) { | |
| return EstimationDeleteOrganization( | |
| unpaidInvoices: List<Invoice>.from( | |
| map['unpaidInvoices'].map((p) => Invoice.fromMap(p))), | |
| ); | |
| } | |
| factory EstimationDeleteOrganization.fromMap(Map<String, dynamic> map) { | |
| return EstimationDeleteOrganization( | |
| unpaidInvoices: List<Invoice>.from( | |
| (map['unpaidInvoices'] ?? []).map((p) => Invoice.fromMap(p))), | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@lib/src/models/estimation_delete_organization.dart` around lines 12 - 17,
EstimationDeleteOrganization.fromMap currently calls
map['unpaidInvoices'].map(...) which will throw if the key is missing or null;
update the factory to defensively handle missing/null by treating
map['unpaidInvoices'] as a list with a fallback (e.g., (map['unpaidInvoices'] ??
[]) ), then safely cast/iterate and map each item via Invoice.fromMap to produce
unpaidInvoices; ensure you use the same pattern as Key.fromMap (default to []
and then List<Invoice>.from(...)) and guard against non-iterable values before
mapping.
| factory PaymentMethodList.fromMap(Map<String, dynamic> map) { | ||
| return PaymentMethodList( | ||
| total: map['total'], | ||
| paymentMethods: List<PaymentMethod>.from( | ||
| map['paymentMethods'].map((p) => PaymentMethod.fromMap(p))), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Same null-safety gap as EstimationDeleteOrganization: no ?? [] guard on map['paymentMethods'].
If the key is absent or null, .map() will throw. Consider adding ?? [] for defensive parsing, consistent with how Key.fromMap handles its list fields.
🤖 Prompt for AI Agents
In `@lib/src/models/payment_method_list.dart` around lines 16 - 22,
PaymentMethodList.fromMap currently calls map['paymentMethods'].map(...) which
will throw if the key is missing or null; update PaymentMethodList.fromMap to
defensively handle a missing/null paymentMethods by using map['paymentMethods']
?? [] and then converting each entry with PaymentMethod.fromMap, ensuring the
resulting paymentMethods is a List<PaymentMethod> (same pattern used in
Key.fromMap) so parsing remains null-safe.
| /// ID of the Team. | ||
| final String userId; | ||
|
|
There was a problem hiding this comment.
Misleading doc comment: userId is documented as "ID of the Team".
The field is named userId but the doc says "ID of the Team." This appears to be a copy-paste error in the API spec.
🤖 Prompt for AI Agents
In `@lib/src/models/payment_method.dart` around lines 26 - 28, The doc comment for
the field userId is incorrect ("ID of the Team"); update the comment above the
declaration of final String userId to accurately describe the field (e.g., "ID
of the user" or "ID of the user who owns this payment method") so the
documentation matches the field name (userId) in the PaymentMethod model.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/services/account.dart`:
- Line 726: The doc comment "Update a new payment method for the current user
account." is misleading; change it to "Update an existing payment method for the
current user account." Update the comment above the updatePaymentMethod method
in lib/services/account.dart (or the AccountService class method that performs
payment method updates) so the text reflects an update of an existing payment
method rather than creating a new one.
🧹 Nitpick comments (1)
lib/models.dart (1)
60-61: Nit:payment_method_list.dartplacement breaks the list-then-model grouping convention.The rest of the file groups
*_list.dartparts at the top (lines 7–16) and singular model parts below (lines 24–59).payment_method_list.dartis placed at the end alongsidepayment_method.dartinstead.Suggested reordering
Move
payment_method_list.dartup into the list group (e.g., after line 23) and keeppayment_method.dartwith the singular models (e.g., after line 59):part 'src/models/locale_code_list.dart'; part 'src/models/transaction_list.dart'; +part 'src/models/payment_method_list.dart'; part 'src/models/row.dart'; ... part 'src/models/subscriber.dart'; part 'src/models/target.dart'; part 'src/models/payment_method.dart'; -part 'src/models/payment_method_list.dart';
| return models.PaymentMethod.fromMap(res.data); | ||
| } | ||
|
|
||
| /// Update a new payment method for the current user account. |
There was a problem hiding this comment.
Doc comment says "Update a new payment method" — should be "Update an existing payment method".
"New" is contradictory with an update operation.
🤖 Prompt for AI Agents
In `@lib/services/account.dart` at line 726, The doc comment "Update a new payment
method for the current user account." is misleading; change it to "Update an
existing payment method for the current user account." Update the comment above
the updatePaymentMethod method in lib/services/account.dart (or the
AccountService class method that performs payment method updates) so the text
reflects an update of an existing payment method rather than creating a new one.
3 participants
This PR contains updates to the Flutter SDK for version 21.1.0.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation