Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughUpdates for SDK version 23.0.0: CHANGELOG, README, and pubspec version/badges updated. Document and Row models change 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 |
* Added extensive docs for `DocumentsDB` and `VectorsDB` usage * Updated README badge to API version `1.9.x` and dependency example * Updated dependency example to `^21.1.0`
ChiragAgg5k
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (9)
lib/src/models/user.dart (1)
113-114: Consider defensive type coercion forimpersonatorfield.Other boolean fields in this model (e.g.,
status,emailVerification,phoneVerification,mfaon lines 102-109) are assigned directly from the map without coercion. However, if the server ever returnsimpersonatoras a non-boolean (e.g.,null,1, or"true"), this would cause a runtime type error.Since
impersonatoris nullable and may not always be present in API responses, consider adding a null check or type coercion for defensive parsing:🛡️ Proposed defensive parsing
- impersonator: map['impersonator'], + impersonator: map['impersonator'] as bool?,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/src/models/user.dart` around lines 113 - 114, The impersonator field in the User model should be parsed defensively to avoid runtime type errors when the server returns non-boolean values; update the assignment for impersonator in the User.fromMap/map constructor (the impersonator field in user.dart) to coerce/string-check nulls and truthy values instead of assigning map['impersonator'] directly — e.g., treat true or "true" (and numeric 1) as true, false or "false" (and 0) as false, and otherwise set null so the nullable type is preserved.docs/examples/documentsdb/create-documents.md (1)
13-13: Consider adding an example document structure.The
documentsparameter is shown as an empty array. For better documentation clarity, consider adding an example document to help users understand the expected structure:documents: [ { 'documentId': '<DOCUMENT_ID>', 'data': { 'name': 'John Doe', 'email': 'john@example.com' } } ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/documentsdb/create-documents.md` at line 13, Replace the empty documents array shown for the documents parameter with a concrete example object so users can see the expected shape: include a sample entry showing the 'documentId' key and a nested 'data' object (e.g., name and email fields) within the 'documents' array so the 'documents' parameter demonstrates the expected structure instead of being empty.docs/examples/documentsdb/update-document.md (1)
12-19: Consider demonstrating the data parameter with example fields.The
dataparameter is shown as an empty object{}(Line 16), which doesn't illustrate what users would typically pass. Adding sample fields would make the example more instructive.📝 Proposed improvement with sample data
Document result = await documentsDB.updateDocument( databaseId: '<DATABASE_ID>', collectionId: '<COLLECTION_ID>', documentId: '<DOCUMENT_ID>', - data: {}, // optional + data: { + 'title': 'Updated Title', + 'status': 'published' + }, // optional permissions: [Permission.read(Role.any())], // optional transactionId: '<TRANSACTION_ID>', // optional );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/documentsdb/update-document.md` around lines 12 - 19, The example for documentsDB.updateDocument currently shows data: {} which is not illustrative; update the example call to provide realistic sample fields (e.g., data containing title, content/body, tags array, lastModified timestamp or metadata map) so readers see the expected shape when calling documentsDB.updateDocument; keep other parameters (databaseId, collectionId, documentId, permissions, transactionId) unchanged and ensure the sample fields match the SDK's document schema expectations.docs/examples/documentsdb/increment-document-attribute.md (1)
10-18: Consider using more meaningful example values.The example uses an empty string for
attribute(Line 14) and0for bothvalueandmax(Lines 15-16). While syntactically valid, these placeholder values don't effectively demonstrate the increment operation. Consider using values likeattribute: 'views',value: 1,max: 1000to better illustrate the feature's purpose.📝 Proposed improvement with illustrative values
Document result = await documentsDB.incrementDocumentAttribute( databaseId: '<DATABASE_ID>', collectionId: '<COLLECTION_ID>', documentId: '<DOCUMENT_ID>', - attribute: '', - value: 0, // optional - max: 0, // optional + attribute: 'views', + value: 1, // optional + max: 1000, // optional transactionId: '<TRANSACTION_ID>', // optional );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/documentsdb/increment-document-attribute.md` around lines 10 - 18, The example call to documentsDB.incrementDocumentAttribute uses unhelpful placeholder values (attribute: '', value: 0, max: 0); update the example invocation of incrementDocumentAttribute (and the returned Document example if present) to use meaningful demonstration values such as attribute: 'views', value: 1, max: 1000 (and keep optional transactionId commented), so readers can clearly see an increment of a 'views' counter by 1 with a sensible cap; ensure parameter names (attribute, value, max, transactionId) remain unchanged.docs/examples/documentsdb/update-transaction.md (1)
10-14: Same feedback as VectorsDB counterpart: clarify mutually exclusive parameters.Consistent with the VectorsDB example, but the same suggestion applies—consider showing a more realistic usage pattern where either
commitorrollbackistrue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/documentsdb/update-transaction.md` around lines 10 - 14, The example for documentsDB.updateTransaction is unclear because it shows both commit and rollback as false; update the example for documentsDB.updateTransaction (transactionId, commit, rollback) to demonstrate a realistic, mutually exclusive usage by setting either commit: true or rollback: true (not both) and add a brief inline note that commit and rollback are mutually exclusive so readers understand the intended pattern.docs/examples/vectorsdb/update-transaction.md (1)
10-14: Consider clarifying the mutually exclusive nature ofcommitandrollback.The example shows both
commit: falseandrollback: false, which is technically valid but may confuse users. In practice, a transaction update would typically set one of these totrue. Consider adding a comment clarifying that these are mutually exclusive options, or showing a more realistic example withcommit: true.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/vectorsdb/update-transaction.md` around lines 10 - 14, Update the example for vectorsDB.updateTransaction to avoid showing both commit and rollback as false and clarify they are mutually exclusive: replace the current call with a realistic example using one action (e.g., commit: true) or provide two short variants (one committing, one rolling back), and add a brief inline comment next to the commit/rollback arguments stating that only one should be true at a time; reference the vectorsDB.updateTransaction call and the transactionId, commit, and rollback parameters when making the change.lib/services/databases.dart (1)
184-214:transactionIdshould use conditional inclusion to avoid sendingnull.Other methods in this file use
if (transactionId != null) 'transactionId': transactionIdpattern (e.g.,listDocumentsat line 136). The current implementation always includestransactionIdin the request, even whennull.♻️ Suggested fix
final Map<String, dynamic> apiParams = { 'documents': documents, - 'transactionId': transactionId, + if (transactionId != null) 'transactionId': transactionId, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/services/databases.dart` around lines 184 - 214, The upsertDocuments method currently always includes 'transactionId' in apiParams even when null; update upsertDocuments to conditionally include transactionId like other methods (e.g., listDocuments) by only adding 'transactionId': transactionId to apiParams when transactionId != null so the request does not send a null value; locate the upsertDocuments function and adjust the construction of apiParams accordingly.docs/examples/documentsdb/decrement-document-attribute.md (1)
14-14: Use a descriptive placeholder forattribute.Empty string
''is a poor placeholder that could mislead users. Other parameters use descriptive placeholders like'<DATABASE_ID>'.📝 Suggested fix
- attribute: '', + attribute: '<ATTRIBUTE_KEY>',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/examples/documentsdb/decrement-document-attribute.md` at line 14, Replace the empty string placeholder for the parameter key "attribute" with a descriptive placeholder (e.g., '<ATTRIBUTE_NAME>') so users understand expected input; locate the example object containing the "attribute" property and update its value from '' to a clear descriptive placeholder consistent with other parameters.lib/services/documents_db.dart (1)
260-277: Reject empty document patches client-side.
transactionIdalone does not update a document. If bothdataandpermissionsare null, this sends an empty PATCH and turns a local usage bug into a runtime API error.Suggested fix
Future<models.Document> updateDocument( {required String databaseId, required String collectionId, required String documentId, Map? data, List<String>? permissions, String? transactionId}) async { + if (data == null && permissions == null) { + throw ArgumentError( + 'At least one of data or permissions must be provided.'); + } + final String apiPath = '/documentsdb/{databaseId}/collections/{collectionId}/documents/{documentId}' .replaceAll('{databaseId}', databaseId)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/services/documents_db.dart` around lines 260 - 277, The updateDocument method builds apiParams for a PATCH but allows sending an empty payload when both data and permissions are null (e.g., only transactionId provided); updateDocument should validate before calling the API and throw or return a descriptive client-side error if neither data nor permissions are present. In the updateDocument function, check that at least one of the fields ('data' or 'permissions') is non-null (or non-empty) before constructing/sending apiParams, and if not, throw an ArgumentError (or custom exception) with a clear message indicating that the patch would be empty; ensure this validation occurs before using apiPath/apiParams so no empty PATCH is sent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/examples/databases/upsert-documents.md`:
- Around line 1-16: The example uses the deprecated Databases.upsertDocuments;
update the snippet to show the current API by replacing Databases and
upsertDocuments with the TablesDB client and TablesDB.upsertRows (adjusting
parameter names: databaseId/collectionId → databaseId/tableId or as required by
TablesDB.upsertRows and documents → rows), or alternatively add a clear
deprecation warning at the top of the example stating that upsertDocuments is
deprecated since v1.8.0 and direct users to TablesDB.upsertRows; reference the
symbols Databases.upsertDocuments and TablesDB.upsertRows so reviewers can
locate and change the example accordingly.
In `@docs/examples/documentsdb/create-operations.md`:
- Around line 12-22: The operations example block uses tab characters for
indentation which is inconsistent with the rest of the document; replace the
tabs inside the operations array (the block containing the "action",
"databaseId", "collectionId", "documentId", and "data" keys) with the same
number of spaces used elsewhere in the file so the array aligns with the
surrounding content and matches the project's spacing conventions.
In `@lib/services/documents_db.dart`:
- Around line 111-112: Update the stale documentation comments in the
DocumentsDb service: replace informal/user-specific phrasing like “the user's
documents”, “row”, and “column” with neutral, API-accurate wording (e.g.,
“documents in a collection”, “document fields” or “document entries”) in the doc
comments attached to the DocumentsDb class and its public listing/query methods
(the comment that currently starts “Get a list of all the user's documents…”
plus the other occurrences that mention “row”/“column”); ensure the comments
describe the method purpose and query parameters concisely and consistently for
IDE tooltips and generated docs.
- Around line 55-63: The updateTransaction method allows commit and rollback to
be both true or both omitted; add a pre-request validation in updateTransaction
that ensures exactly one action is requested: if both commit and rollback are
null or if both are true, throw or return a descriptive error (e.g.,
ArgumentError) before building apiParams or calling the API; reference the
updateTransaction function and the apiParams construction so the check runs
immediately after the parameters are received and before the request is made.
- Around line 91-99: The createOperations method currently accepts a nullable
List<Map>? operations which allows callers to send empty/null create requests;
change the signature of createOperations to require a non-null List<Map>
operations (remove the ? and add required), and add an early validation in the
method (e.g., if (operations.isEmpty) throw ArgumentError('operations must not
be empty')) to reject empty payloads before calling the API; update the
construction of apiParams to always include 'operations': operations and keep
references to the existing transactionId replacement and return type
models.Transaction unchanged.
In `@lib/src/client_browser.dart`:
- Around line 98-120: The impersonation setters (setImpersonateUserId,
setImpersonateUserEmail, setImpersonateUserPhone) leave previous
X-Appwrite-Impersonate-User-* headers and config keys, allowing conflicting
impersonation data to leak between requests; update each setter to clear the
other impersonation entries before setting its own value (remove the other
config keys e.g.
'impersonateUserId'/'impersonateUserEmail'/'impersonateUserPhone' and delete the
corresponding headers
'X-Appwrite-Impersonate-User-Id'/'X-Appwrite-Impersonate-User-Email'/'X-Appwrite-Impersonate-User-Phone'
from the internal headers map), then add the new header and config; apply the
same change to the mirrored implementations in ClientIO (the methods in
lib/src/client_io.dart) so switching between ID/email/phone never leaves stale
impersonation headers or config.
In `@README.md`:
- Around line 5-10: The README's server compatibility statement is out of sync
with the API version badge; update the compatibility sentence that currently
reads "This SDK is compatible with Appwrite server version 1.8.x" to match the
badge (e.g., "1.9.x") or explicitly explain mapping between API and server
versions; edit the README.md paragraph containing that sentence so the declared
server version aligns with the API badge (or add a short clarifying note about
differences).
- Line 22: Replace the bogus dependency line "appwrite: ^0.0.0" in the README
with the correct version constraint for the Appwrite package (e.g., "appwrite:
^21.1.1") so it matches the actual declared package version; update the README's
dependency entry "appwrite: ^0.0.0" to "appwrite: ^21.1.1" (or an appropriate
compatible range) and verify it aligns with the package manifest that declares
the real version.
In `@test/services/documents_db_test.dart`:
- Around line 10-47: The MockClient's overrides (call, webAuth, chunkedUpload)
drop named parameters when delegating to noSuchMethod, so tests can't assert on
path/headers/params/responseType (and callbackUrlScheme/onProgress); update each
override in class MockClient to forward named arguments to noSuchMethod by
passing a namedArguments map (keys as Symbol('path'), Symbol('headers'), etc.)
in the third parameter of Invocation.method for call, webAuth, and chunkedUpload
so the mock records and matches both positional and named args correctly.
In `@test/services/vectors_db_test.dart`:
- Around line 10-47: The mock's noSuchMethod calls only pass positional args, so
named arguments are ignored and matchers like when(client.call(HttpMethod.*))
succeed regardless of path/params; update MockClient to forward named arguments
into Invocation.method for call, webAuth, and chunkedUpload by supplying the
optional namedArguments Map<Symbol, dynamic> (e.g., Symbol('path'),
Symbol('headers'), Symbol('params'), Symbol('responseType') for call;
Symbol('callbackUrlScheme') for webAuth; and Symbol('path'), Symbol('params'),
Symbol('paramName'), Symbol('idParamName'), Symbol('headers'),
Symbol('onProgress') for chunkedUpload) so the mock records named parameters
correctly and tests can assert on them.
---
Nitpick comments:
In `@docs/examples/documentsdb/create-documents.md`:
- Line 13: Replace the empty documents array shown for the documents parameter
with a concrete example object so users can see the expected shape: include a
sample entry showing the 'documentId' key and a nested 'data' object (e.g., name
and email fields) within the 'documents' array so the 'documents' parameter
demonstrates the expected structure instead of being empty.
In `@docs/examples/documentsdb/decrement-document-attribute.md`:
- Line 14: Replace the empty string placeholder for the parameter key
"attribute" with a descriptive placeholder (e.g., '<ATTRIBUTE_NAME>') so users
understand expected input; locate the example object containing the "attribute"
property and update its value from '' to a clear descriptive placeholder
consistent with other parameters.
In `@docs/examples/documentsdb/increment-document-attribute.md`:
- Around line 10-18: The example call to documentsDB.incrementDocumentAttribute
uses unhelpful placeholder values (attribute: '', value: 0, max: 0); update the
example invocation of incrementDocumentAttribute (and the returned Document
example if present) to use meaningful demonstration values such as attribute:
'views', value: 1, max: 1000 (and keep optional transactionId commented), so
readers can clearly see an increment of a 'views' counter by 1 with a sensible
cap; ensure parameter names (attribute, value, max, transactionId) remain
unchanged.
In `@docs/examples/documentsdb/update-document.md`:
- Around line 12-19: The example for documentsDB.updateDocument currently shows
data: {} which is not illustrative; update the example call to provide realistic
sample fields (e.g., data containing title, content/body, tags array,
lastModified timestamp or metadata map) so readers see the expected shape when
calling documentsDB.updateDocument; keep other parameters (databaseId,
collectionId, documentId, permissions, transactionId) unchanged and ensure the
sample fields match the SDK's document schema expectations.
In `@docs/examples/documentsdb/update-transaction.md`:
- Around line 10-14: The example for documentsDB.updateTransaction is unclear
because it shows both commit and rollback as false; update the example for
documentsDB.updateTransaction (transactionId, commit, rollback) to demonstrate a
realistic, mutually exclusive usage by setting either commit: true or rollback:
true (not both) and add a brief inline note that commit and rollback are
mutually exclusive so readers understand the intended pattern.
In `@docs/examples/vectorsdb/update-transaction.md`:
- Around line 10-14: Update the example for vectorsDB.updateTransaction to avoid
showing both commit and rollback as false and clarify they are mutually
exclusive: replace the current call with a realistic example using one action
(e.g., commit: true) or provide two short variants (one committing, one rolling
back), and add a brief inline comment next to the commit/rollback arguments
stating that only one should be true at a time; reference the
vectorsDB.updateTransaction call and the transactionId, commit, and rollback
parameters when making the change.
In `@lib/services/databases.dart`:
- Around line 184-214: The upsertDocuments method currently always includes
'transactionId' in apiParams even when null; update upsertDocuments to
conditionally include transactionId like other methods (e.g., listDocuments) by
only adding 'transactionId': transactionId to apiParams when transactionId !=
null so the request does not send a null value; locate the upsertDocuments
function and adjust the construction of apiParams accordingly.
In `@lib/services/documents_db.dart`:
- Around line 260-277: The updateDocument method builds apiParams for a PATCH
but allows sending an empty payload when both data and permissions are null
(e.g., only transactionId provided); updateDocument should validate before
calling the API and throw or return a descriptive client-side error if neither
data nor permissions are present. In the updateDocument function, check that at
least one of the fields ('data' or 'permissions') is non-null (or non-empty)
before constructing/sending apiParams, and if not, throw an ArgumentError (or
custom exception) with a clear message indicating that the patch would be empty;
ensure this validation occurs before using apiPath/apiParams so no empty PATCH
is sent.
In `@lib/src/models/user.dart`:
- Around line 113-114: The impersonator field in the User model should be parsed
defensively to avoid runtime type errors when the server returns non-boolean
values; update the assignment for impersonator in the User.fromMap/map
constructor (the impersonator field in user.dart) to coerce/string-check nulls
and truthy values instead of assigning map['impersonator'] directly — e.g.,
treat true or "true" (and numeric 1) as true, false or "false" (and 0) as false,
and otherwise set null so the nullable type is preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7d0576ec-be5f-44cc-a157-f341d64d503a
📒 Files selected for processing (49)
CHANGELOG.mdREADME.mddocs/examples/databases/upsert-documents.mddocs/examples/documentsdb/create-document.mddocs/examples/documentsdb/create-documents.mddocs/examples/documentsdb/create-operations.mddocs/examples/documentsdb/create-transaction.mddocs/examples/documentsdb/decrement-document-attribute.mddocs/examples/documentsdb/delete-document.mddocs/examples/documentsdb/delete-transaction.mddocs/examples/documentsdb/get-document.mddocs/examples/documentsdb/get-transaction.mddocs/examples/documentsdb/increment-document-attribute.mddocs/examples/documentsdb/list-documents.mddocs/examples/documentsdb/list-transactions.mddocs/examples/documentsdb/update-document.mddocs/examples/documentsdb/update-transaction.mddocs/examples/documentsdb/upsert-document.mddocs/examples/vectorsdb/create-document.mddocs/examples/vectorsdb/create-operations.mddocs/examples/vectorsdb/create-transaction.mddocs/examples/vectorsdb/delete-document.mddocs/examples/vectorsdb/delete-transaction.mddocs/examples/vectorsdb/get-document.mddocs/examples/vectorsdb/get-transaction.mddocs/examples/vectorsdb/list-documents.mddocs/examples/vectorsdb/list-transactions.mddocs/examples/vectorsdb/update-document.mddocs/examples/vectorsdb/update-transaction.mddocs/examples/vectorsdb/upsert-document.mdlib/appwrite.dartlib/services/databases.dartlib/services/documents_db.dartlib/services/vectors_db.dartlib/src/client.dartlib/src/client_base.dartlib/src/client_browser.dartlib/src/client_io.dartlib/src/models/document.dartlib/src/models/log.dartlib/src/models/row.dartlib/src/models/user.dartpubspec.yamltest/services/databases_test.darttest/services/documents_db_test.darttest/services/tables_db_test.darttest/services/vectors_db_test.darttest/src/models/document_test.darttest/src/models/row_test.dart
Comment on lines
+12
to
+22
| operations: [ | ||
| { | ||
| "action": "create", | ||
| "databaseId": "<DATABASE_ID>", | ||
| "collectionId": "<COLLECTION_ID>", | ||
| "documentId": "<DOCUMENT_ID>", | ||
| "data": { | ||
| "name": "Walter O'Brien" | ||
| } | ||
| } | ||
| ], // optional |
There was a problem hiding this comment.
Inconsistent indentation: tabs vs spaces.
The operations array content (lines 13-21) uses tab indentation while the rest of the example uses spaces. This inconsistency may cause rendering issues in some Markdown viewers and doesn't follow the formatting pattern of other examples in this PR.
Proposed fix to use consistent spacing
operations: [
- {
- "action": "create",
- "databaseId": "<DATABASE_ID>",
- "collectionId": "<COLLECTION_ID>",
- "documentId": "<DOCUMENT_ID>",
- "data": {
- "name": "Walter O'Brien"
- }
- }
- ], // optional
+ {
+ "action": "create",
+ "databaseId": "<DATABASE_ID>",
+ "collectionId": "<COLLECTION_ID>",
+ "documentId": "<DOCUMENT_ID>",
+ "data": {
+ "name": "Walter O'Brien"
+ }
+ }
+ ], // optional📝 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.
Suggested change
| operations: [ | |
| { | |
| "action": "create", | |
| "databaseId": "<DATABASE_ID>", | |
| "collectionId": "<COLLECTION_ID>", | |
| "documentId": "<DOCUMENT_ID>", | |
| "data": { | |
| "name": "Walter O'Brien" | |
| } | |
| } | |
| ], // optional | |
| operations: [ | |
| { | |
| "action": "create", | |
| "databaseId": "<DATABASE_ID>", | |
| "collectionId": "<COLLECTION_ID>", | |
| "documentId": "<DOCUMENT_ID>", | |
| "data": { | |
| "name": "Walter O'Brien" | |
| } | |
| } | |
| ], // optional |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/examples/documentsdb/create-operations.md` around lines 12 - 22, The
operations example block uses tab characters for indentation which is
inconsistent with the rest of the document; replace the tabs inside the
operations array (the block containing the "action", "databaseId",
"collectionId", "documentId", and "data" keys) with the same number of spaces
used elsewhere in the file so the array aligns with the surrounding content and
matches the project's spacing conventions.
lib/services/documents_db.dart
Outdated
Comment on lines
+55
to
+63
| Future<models.Transaction> updateTransaction( | ||
| {required String transactionId, bool? commit, bool? rollback}) async { | ||
| final String apiPath = '/documentsdb/transactions/{transactionId}' | ||
| .replaceAll('{transactionId}', transactionId); | ||
|
|
||
| final Map<String, dynamic> apiParams = { | ||
| if (commit != null) 'commit': commit, | ||
| if (rollback != null) 'rollback': rollback, | ||
| }; |
There was a problem hiding this comment.
Require exactly one transaction action.
updateTransaction() currently permits both commit and rollback to be true, or neither to be set. Both cases are invalid/ambiguous for this API and should fail before the request is sent.
Suggested fix
Future<models.Transaction> updateTransaction(
{required String transactionId, bool? commit, bool? rollback}) async {
+ final actionCount =
+ [commit == true, rollback == true].where((v) => v).length;
+ if (actionCount != 1) {
+ throw ArgumentError(
+ 'Exactly one of commit or rollback must be true.');
+ }
+
final String apiPath = '/documentsdb/transactions/{transactionId}'
.replaceAll('{transactionId}', transactionId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/services/documents_db.dart` around lines 55 - 63, The updateTransaction
method allows commit and rollback to be both true or both omitted; add a
pre-request validation in updateTransaction that ensures exactly one action is
requested: if both commit and rollback are null or if both are true, throw or
return a descriptive error (e.g., ArgumentError) before building apiParams or
calling the API; reference the updateTransaction function and the apiParams
construction so the check runs immediately after the parameters are received and
before the request is made.
lib/services/documents_db.dart
Outdated
Comment on lines
+91
to
+99
| Future<models.Transaction> createOperations( | ||
| {required String transactionId, List<Map>? operations}) async { | ||
| final String apiPath = | ||
| '/documentsdb/transactions/{transactionId}/operations' | ||
| .replaceAll('{transactionId}', transactionId); | ||
|
|
||
| final Map<String, dynamic> apiParams = { | ||
| if (operations != null) 'operations': operations, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Make operations required on createOperations().
Allowing null here exposes an empty “create” request in the public SDK surface and defers a caller error to the API response. This should be required, and ideally rejected when empty.
Suggested fix
Future<models.Transaction> createOperations(
- {required String transactionId, List<Map>? operations}) async {
+ {required String transactionId,
+ required List<Map<String, dynamic>> operations}) async {
+ if (operations.isEmpty) {
+ throw ArgumentError.value(
+ operations, 'operations', 'Must contain at least one operation.');
+ }
+
final String apiPath =
'/documentsdb/transactions/{transactionId}/operations'
.replaceAll('{transactionId}', transactionId);
final Map<String, dynamic> apiParams = {
- if (operations != null) 'operations': operations,
+ 'operations': operations,
};📝 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.
Suggested change
| Future<models.Transaction> createOperations( | |
| {required String transactionId, List<Map>? operations}) async { | |
| final String apiPath = | |
| '/documentsdb/transactions/{transactionId}/operations' | |
| .replaceAll('{transactionId}', transactionId); | |
| final Map<String, dynamic> apiParams = { | |
| if (operations != null) 'operations': operations, | |
| }; | |
| Future<models.Transaction> createOperations( | |
| {required String transactionId, | |
| required List<Map<String, dynamic>> operations}) async { | |
| if (operations.isEmpty) { | |
| throw ArgumentError.value( | |
| operations, 'operations', 'Must contain at least one operation.'); | |
| } | |
| final String apiPath = | |
| '/documentsdb/transactions/{transactionId}/operations' | |
| .replaceAll('{transactionId}', transactionId); | |
| final Map<String, dynamic> apiParams = { | |
| 'operations': operations, | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/services/documents_db.dart` around lines 91 - 99, The createOperations
method currently accepts a nullable List<Map>? operations which allows callers
to send empty/null create requests; change the signature of createOperations to
require a non-null List<Map> operations (remove the ? and add required), and add
an early validation in the method (e.g., if (operations.isEmpty) throw
ArgumentError('operations must not be empty')) to reject empty payloads before
calling the API; update the construction of apiParams to always include
'operations': operations and keep references to the existing transactionId
replacement and return type models.Transaction unchanged.
lib/services/documents_db.dart
Outdated
Comment on lines
+111
to
+112
| /// Get a list of all the user's documents in a given collection. You can use | ||
| /// the query params to filter your results. |
There was a problem hiding this comment.
Fix copied/stale docs before release.
These comments still talk about “the user's documents”, “row”, and “column”. They will show up in IDE tooltips and generated API docs for a document service.
Suggested wording
- /// Get a list of all the user's documents in a given collection. You can use
+ /// Get a list of documents in a given collection. You can use
/// the query params to filter your results.
...
- /// Decrement a specific column of a row by a given value.
+ /// Decrement a specific attribute of a document by a given value.
...
- /// Increment a specific column of a row by a given value.
+ /// Increment a specific attribute of a document by a given value.Also applies to: 315-315, 347-347
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/services/documents_db.dart` around lines 111 - 112, Update the stale
documentation comments in the DocumentsDb service: replace
informal/user-specific phrasing like “the user's documents”, “row”, and “column”
with neutral, API-accurate wording (e.g., “documents in a collection”, “document
fields” or “document entries”) in the doc comments attached to the DocumentsDb
class and its public listing/query methods (the comment that currently starts
“Get a list of all the user's documents…” plus the other occurrences that
mention “row”/“column”); ensure the comments describe the method purpose and
query parameters concisely and consistently for IDE tooltips and generated docs.
Comment on lines
+98
to
+120
| /// Impersonate a user by ID on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data. | ||
| @override | ||
| ClientBrowser setImpersonateUserId(value) { | ||
| config['impersonateUserId'] = value; | ||
| addHeader('X-Appwrite-Impersonate-User-Id', value); | ||
| return this; | ||
| } | ||
|
|
||
| /// Impersonate a user by email on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data. | ||
| @override | ||
| ClientBrowser setImpersonateUserEmail(value) { | ||
| config['impersonateUserEmail'] = value; | ||
| addHeader('X-Appwrite-Impersonate-User-Email', value); | ||
| return this; | ||
| } | ||
|
|
||
| /// Impersonate a user by phone on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data. | ||
| @override | ||
| ClientBrowser setImpersonateUserPhone(value) { | ||
| config['impersonateUserPhone'] = value; | ||
| addHeader('X-Appwrite-Impersonate-User-Phone', value); | ||
| return this; | ||
| } |
There was a problem hiding this comment.
Avoid leaking multiple impersonation selectors across requests.
These setters only add their own header/config entry. If the same client instance switches from ID to email or phone, the earlier X-Appwrite-Impersonate-User-* headers remain in _headers, so later requests can carry conflicting impersonation targets. The mirrored implementation in lib/src/client_io.dart:124-146 has the same problem.
Suggested fix
+ void _clearImpersonation() {
+ config
+ ..remove('impersonateUserId')
+ ..remove('impersonateUserEmail')
+ ..remove('impersonateUserPhone');
+ _headers!
+ ..remove('X-Appwrite-Impersonate-User-Id')
+ ..remove('X-Appwrite-Impersonate-User-Email')
+ ..remove('X-Appwrite-Impersonate-User-Phone');
+ }
+
/// Impersonate a user by ID on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data.
`@override`
ClientBrowser setImpersonateUserId(value) {
+ _clearImpersonation();
config['impersonateUserId'] = value;
addHeader('X-Appwrite-Impersonate-User-Id', value);
return this;
}
@@
`@override`
ClientBrowser setImpersonateUserEmail(value) {
+ _clearImpersonation();
config['impersonateUserEmail'] = value;
addHeader('X-Appwrite-Impersonate-User-Email', value);
return this;
}
@@
`@override`
ClientBrowser setImpersonateUserPhone(value) {
+ _clearImpersonation();
config['impersonateUserPhone'] = value;
addHeader('X-Appwrite-Impersonate-User-Phone', value);
return this;
}📝 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.
Suggested change
| /// Impersonate a user by ID on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data. | |
| @override | |
| ClientBrowser setImpersonateUserId(value) { | |
| config['impersonateUserId'] = value; | |
| addHeader('X-Appwrite-Impersonate-User-Id', value); | |
| return this; | |
| } | |
| /// Impersonate a user by email on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data. | |
| @override | |
| ClientBrowser setImpersonateUserEmail(value) { | |
| config['impersonateUserEmail'] = value; | |
| addHeader('X-Appwrite-Impersonate-User-Email', value); | |
| return this; | |
| } | |
| /// Impersonate a user by phone on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data. | |
| @override | |
| ClientBrowser setImpersonateUserPhone(value) { | |
| config['impersonateUserPhone'] = value; | |
| addHeader('X-Appwrite-Impersonate-User-Phone', value); | |
| return this; | |
| } | |
| void _clearImpersonation() { | |
| config | |
| ..remove('impersonateUserId') | |
| ..remove('impersonateUserEmail') | |
| ..remove('impersonateUserPhone'); | |
| _headers! | |
| ..remove('X-Appwrite-Impersonate-User-Id') | |
| ..remove('X-Appwrite-Impersonate-User-Email') | |
| ..remove('X-Appwrite-Impersonate-User-Phone'); | |
| } | |
| /// Impersonate a user by ID on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data. | |
| `@override` | |
| ClientBrowser setImpersonateUserId(value) { | |
| _clearImpersonation(); | |
| config['impersonateUserId'] = value; | |
| addHeader('X-Appwrite-Impersonate-User-Id', value); | |
| return this; | |
| } | |
| /// Impersonate a user by email on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data. | |
| `@override` | |
| ClientBrowser setImpersonateUserEmail(value) { | |
| _clearImpersonation(); | |
| config['impersonateUserEmail'] = value; | |
| addHeader('X-Appwrite-Impersonate-User-Email', value); | |
| return this; | |
| } | |
| /// Impersonate a user by phone on an already user-authenticated request. Requires the current request to be authenticated as a user with impersonator capability; X-Appwrite-Key alone is not sufficient. Impersonator users are intentionally granted users.read so they can discover a target before impersonation begins. Internal audit logs still attribute actions to the original impersonator and record the impersonated target only in internal audit payload data. | |
| `@override` | |
| ClientBrowser setImpersonateUserPhone(value) { | |
| _clearImpersonation(); | |
| config['impersonateUserPhone'] = value; | |
| addHeader('X-Appwrite-Impersonate-User-Phone', value); | |
| return this; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/src/client_browser.dart` around lines 98 - 120, The impersonation setters
(setImpersonateUserId, setImpersonateUserEmail, setImpersonateUserPhone) leave
previous X-Appwrite-Impersonate-User-* headers and config keys, allowing
conflicting impersonation data to leak between requests; update each setter to
clear the other impersonation entries before setting its own value (remove the
other config keys e.g.
'impersonateUserId'/'impersonateUserEmail'/'impersonateUserPhone' and delete the
corresponding headers
'X-Appwrite-Impersonate-User-Id'/'X-Appwrite-Impersonate-User-Email'/'X-Appwrite-Impersonate-User-Phone'
from the internal headers map), then add the new header and config; apply the
same change to the mirrored implementations in ClientIO (the methods in
lib/src/client_io.dart) so switching between ID/email/phone never leaves stale
impersonation headers or config.
test/services/documents_db_test.dart
Outdated
Comment on lines
+10
to
+47
| class MockClient extends Mock implements Client { | ||
| Map<String, String> config = {'project': 'testproject'}; | ||
| String endPoint = 'https://localhost/v1'; | ||
| @override | ||
| Future<Response> call( | ||
| HttpMethod? method, { | ||
| String path = '', | ||
| Map<String, String> headers = const {}, | ||
| Map<String, dynamic> params = const {}, | ||
| ResponseType? responseType, | ||
| }) async { | ||
| return super.noSuchMethod(Invocation.method(#call, [method]), | ||
| returnValue: Response()); | ||
| } | ||
|
|
||
| @override | ||
| Future webAuth( | ||
| Uri? url, { | ||
| String? callbackUrlScheme, | ||
| }) async { | ||
| return super | ||
| .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | ||
| } | ||
|
|
||
| @override | ||
| Future<Response> chunkedUpload({ | ||
| String? path, | ||
| Map<String, dynamic>? params, | ||
| String? paramName, | ||
| String? idParamName, | ||
| Map<String, String>? headers, | ||
| Function(UploadProgress)? onProgress, | ||
| }) async { | ||
| return super.noSuchMethod( | ||
| Invocation.method( | ||
| #chunkedUpload, [path, params, paramName, idParamName, headers]), | ||
| returnValue: Response(data: {})); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C2 'Invocation\.method\(#(call|webAuth|chunkedUpload), \[' testRepository: appwrite/sdk-for-flutter
Length of output: 12295
🏁 Script executed:
head -50 test/services/documents_db_test.dart | tail -40Repository: appwrite/sdk-for-flutter
Length of output: 1161
Forward named arguments through noSuchMethod.
Invocation.method(#call, [method]) drops path, headers, params, and responseType, so a malformed request still satisfies when(client.call(HttpMethod.*)). This blinds the entire test suite to endpoint and payload regressions. The same issue affects webAuth and chunkedUpload.
This pattern appears in at least 12 test files across test/services/. Either fix all three methods to forward named arguments via Invocation.method's third parameter, or use a generated Mockito mock instead of this manual implementation.
Representative fix for the `call` method
`@override`
Future<Response> call(
- HttpMethod? method, {
+ HttpMethod method, {
String path = '',
Map<String, String> headers = const {},
Map<String, dynamic> params = const {},
ResponseType? responseType,
- }) async {
- return super.noSuchMethod(Invocation.method(`#call`, [method]),
- returnValue: Response());
+ }) {
+ return super.noSuchMethod(
+ Invocation.method(`#call`, [method], {
+ `#path`: path,
+ `#headers`: headers,
+ `#params`: params,
+ `#responseType`: responseType,
+ }),
+ returnValue: Future.value(Response()),
+ returnValueForMissingStub: Future.value(Response()),
+ ) as Future<Response>;
}Apply the same pattern to webAuth and chunkedUpload.
📝 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.
Suggested change
| class MockClient extends Mock implements Client { | |
| Map<String, String> config = {'project': 'testproject'}; | |
| String endPoint = 'https://localhost/v1'; | |
| @override | |
| Future<Response> call( | |
| HttpMethod? method, { | |
| String path = '', | |
| Map<String, String> headers = const {}, | |
| Map<String, dynamic> params = const {}, | |
| ResponseType? responseType, | |
| }) async { | |
| return super.noSuchMethod(Invocation.method(#call, [method]), | |
| returnValue: Response()); | |
| } | |
| @override | |
| Future webAuth( | |
| Uri? url, { | |
| String? callbackUrlScheme, | |
| }) async { | |
| return super | |
| .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | |
| } | |
| @override | |
| Future<Response> chunkedUpload({ | |
| String? path, | |
| Map<String, dynamic>? params, | |
| String? paramName, | |
| String? idParamName, | |
| Map<String, String>? headers, | |
| Function(UploadProgress)? onProgress, | |
| }) async { | |
| return super.noSuchMethod( | |
| Invocation.method( | |
| #chunkedUpload, [path, params, paramName, idParamName, headers]), | |
| returnValue: Response(data: {})); | |
| } | |
| class MockClient extends Mock implements Client { | |
| Map<String, String> config = {'project': 'testproject'}; | |
| String endPoint = 'https://localhost/v1'; | |
| `@override` | |
| Future<Response> call( | |
| HttpMethod method, { | |
| String path = '', | |
| Map<String, String> headers = const {}, | |
| Map<String, dynamic> params = const {}, | |
| ResponseType? responseType, | |
| }) { | |
| return super.noSuchMethod( | |
| Invocation.method(`#call`, [method], { | |
| `#path`: path, | |
| `#headers`: headers, | |
| `#params`: params, | |
| `#responseType`: responseType, | |
| }), | |
| returnValue: Future.value(Response()), | |
| returnValueForMissingStub: Future.value(Response()), | |
| ) as Future<Response>; | |
| } | |
| `@override` | |
| Future webAuth( | |
| Uri? url, { | |
| String? callbackUrlScheme, | |
| }) async { | |
| return super | |
| .noSuchMethod(Invocation.method(`#webAuth`, [url]), returnValue: 'done'); | |
| } | |
| `@override` | |
| Future<Response> chunkedUpload({ | |
| String? path, | |
| Map<String, dynamic>? params, | |
| String? paramName, | |
| String? idParamName, | |
| Map<String, String>? headers, | |
| Function(UploadProgress)? onProgress, | |
| }) async { | |
| return super.noSuchMethod( | |
| Invocation.method( | |
| `#chunkedUpload`, [path, params, paramName, idParamName, headers]), | |
| returnValue: Response(data: {})); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/services/documents_db_test.dart` around lines 10 - 47, The MockClient's
overrides (call, webAuth, chunkedUpload) drop named parameters when delegating
to noSuchMethod, so tests can't assert on path/headers/params/responseType (and
callbackUrlScheme/onProgress); update each override in class MockClient to
forward named arguments to noSuchMethod by passing a namedArguments map (keys as
Symbol('path'), Symbol('headers'), etc.) in the third parameter of
Invocation.method for call, webAuth, and chunkedUpload so the mock records and
matches both positional and named args correctly.
test/services/vectors_db_test.dart
Outdated
Comment on lines
+10
to
+47
| class MockClient extends Mock implements Client { | ||
| Map<String, String> config = {'project': 'testproject'}; | ||
| String endPoint = 'https://localhost/v1'; | ||
| @override | ||
| Future<Response> call( | ||
| HttpMethod? method, { | ||
| String path = '', | ||
| Map<String, String> headers = const {}, | ||
| Map<String, dynamic> params = const {}, | ||
| ResponseType? responseType, | ||
| }) async { | ||
| return super.noSuchMethod(Invocation.method(#call, [method]), | ||
| returnValue: Response()); | ||
| } | ||
|
|
||
| @override | ||
| Future webAuth( | ||
| Uri? url, { | ||
| String? callbackUrlScheme, | ||
| }) async { | ||
| return super | ||
| .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | ||
| } | ||
|
|
||
| @override | ||
| Future<Response> chunkedUpload({ | ||
| String? path, | ||
| Map<String, dynamic>? params, | ||
| String? paramName, | ||
| String? idParamName, | ||
| Map<String, String>? headers, | ||
| Function(UploadProgress)? onProgress, | ||
| }) async { | ||
| return super.noSuchMethod( | ||
| Invocation.method( | ||
| #chunkedUpload, [path, params, paramName, idParamName, headers]), | ||
| returnValue: Response(data: {})); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C2 'Invocation\.method\(#(call|webAuth|chunkedUpload), \[' testRepository: appwrite/sdk-for-flutter
Length of output: 12295
Forward named arguments through noSuchMethod.
The mock only records the HTTP verb, so when(client.call(HttpMethod.*)) still matches regardless of path or params passed to VectorsDB. This affects coverage across multiple service test files using the same pattern. For a thin request-builder service, this leaves tests unable to catch wrong paths, missing IDs, or malformed request parameters.
Representative fix
class MockClient extends Mock implements Client {
Map<String, String> config = {'project': 'testproject'};
String endPoint = 'https://localhost/v1';
`@override`
Future<Response> call(
- HttpMethod? method, {
+ HttpMethod method, {
String path = '',
Map<String, String> headers = const {},
Map<String, dynamic> params = const {},
ResponseType? responseType,
- }) async {
- return super.noSuchMethod(Invocation.method(`#call`, [method]),
- returnValue: Response());
+ }) {
+ return super.noSuchMethod(
+ Invocation.method(`#call`, [method], {
+ `#path`: path,
+ `#headers`: headers,
+ `#params`: params,
+ `#responseType`: responseType,
+ }),
+ returnValue: Future.value(Response()),
+ returnValueForMissingStub: Future.value(Response()),
+ ) as Future<Response>;
}Apply the same pattern to webAuth and chunkedUpload. This same issue exists across all service test files (avatars, teams, tables_db, storage, messaging, locale, graphql, functions, documents_db, databases, account).
📝 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.
Suggested change
| class MockClient extends Mock implements Client { | |
| Map<String, String> config = {'project': 'testproject'}; | |
| String endPoint = 'https://localhost/v1'; | |
| @override | |
| Future<Response> call( | |
| HttpMethod? method, { | |
| String path = '', | |
| Map<String, String> headers = const {}, | |
| Map<String, dynamic> params = const {}, | |
| ResponseType? responseType, | |
| }) async { | |
| return super.noSuchMethod(Invocation.method(#call, [method]), | |
| returnValue: Response()); | |
| } | |
| @override | |
| Future webAuth( | |
| Uri? url, { | |
| String? callbackUrlScheme, | |
| }) async { | |
| return super | |
| .noSuchMethod(Invocation.method(#webAuth, [url]), returnValue: 'done'); | |
| } | |
| @override | |
| Future<Response> chunkedUpload({ | |
| String? path, | |
| Map<String, dynamic>? params, | |
| String? paramName, | |
| String? idParamName, | |
| Map<String, String>? headers, | |
| Function(UploadProgress)? onProgress, | |
| }) async { | |
| return super.noSuchMethod( | |
| Invocation.method( | |
| #chunkedUpload, [path, params, paramName, idParamName, headers]), | |
| returnValue: Response(data: {})); | |
| } | |
| class MockClient extends Mock implements Client { | |
| Map<String, String> config = {'project': 'testproject'}; | |
| String endPoint = 'https://localhost/v1'; | |
| `@override` | |
| Future<Response> call( | |
| HttpMethod method, { | |
| String path = '', | |
| Map<String, String> headers = const {}, | |
| Map<String, dynamic> params = const {}, | |
| ResponseType? responseType, | |
| }) { | |
| return super.noSuchMethod( | |
| Invocation.method(`#call`, [method], { | |
| `#path`: path, | |
| `#headers`: headers, | |
| `#params`: params, | |
| `#responseType`: responseType, | |
| }), | |
| returnValue: Future.value(Response()), | |
| returnValueForMissingStub: Future.value(Response()), | |
| ) as Future<Response>; | |
| } | |
| `@override` | |
| Future webAuth( | |
| Uri? url, { | |
| String? callbackUrlScheme, | |
| }) async { | |
| return super | |
| .noSuchMethod(Invocation.method(`#webAuth`, [url]), returnValue: 'done'); | |
| } | |
| `@override` | |
| Future<Response> chunkedUpload({ | |
| String? path, | |
| Map<String, dynamic>? params, | |
| String? paramName, | |
| String? idParamName, | |
| Map<String, String>? headers, | |
| Function(UploadProgress)? onProgress, | |
| }) async { | |
| return super.noSuchMethod( | |
| Invocation.method( | |
| `#chunkedUpload`, [path, params, paramName, idParamName, headers]), | |
| returnValue: Response(data: {})); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/services/vectors_db_test.dart` around lines 10 - 47, The mock's
noSuchMethod calls only pass positional args, so named arguments are ignored and
matchers like when(client.call(HttpMethod.*)) succeed regardless of path/params;
update MockClient to forward named arguments into Invocation.method for call,
webAuth, and chunkedUpload by supplying the optional namedArguments Map<Symbol,
dynamic> (e.g., Symbol('path'), Symbol('headers'), Symbol('params'),
Symbol('responseType') for call; Symbol('callbackUrlScheme') for webAuth; and
Symbol('path'), Symbol('params'), Symbol('paramName'), Symbol('idParamName'),
Symbol('headers'), Symbol('onProgress') for chunkedUpload) so the mock records
named parameters correctly and tests can assert on them.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
README.md (1)
5-10:⚠️ Potential issue | 🟠 MajorFix version inconsistency between badge and compatibility statement.
The API version badge displays
1.9.x(Line 5), but the compatibility statement declares support for server version1.8.x(Line 10). According to the library header inlib/appwrite.dart, the SDK is compatible with "Appwrite server version 1.9.x". Update Line 10 to state1.9.xto match both the badge and the library documentation.📝 Proposed fix
-**This SDK is compatible with Appwrite server version 1.8.x. For older versions, please check [previous releases](https://github.com/appwrite/sdk-for-flutter/releases).** +**This SDK is compatible with Appwrite server version 1.9.x. For older versions, please check [previous releases](https://github.com/appwrite/sdk-for-flutter/releases).**🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 5 - 10, Update the compatibility statement in README.md to match the API version badge and the lib/appwrite.dart header by changing the server version text from "1.8.x" to "1.9.x" so the sentence "**This SDK is compatible with Appwrite server version 1.8.x.**" reads "**This SDK is compatible with Appwrite server version 1.9.x.**"; ensure consistency with the Version badge (1.9.x) and the lib/appwrite.dart library header.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 22: The CHANGELOG entry for version 21.1.1 mentions "Updated dependency
example to `^21.1.0`" which is inconsistent with the README showing `appwrite:
^21.1.1`; update the CHANGELOG text for version 21.1.1 to reference `^21.1.1`
instead of `^21.1.0` so both documents match (search for the CHANGELOG entry
labeled 21.1.1 and replace the dependency example version).
---
Duplicate comments:
In `@README.md`:
- Around line 5-10: Update the compatibility statement in README.md to match the
API version badge and the lib/appwrite.dart header by changing the server
version text from "1.8.x" to "1.9.x" so the sentence "**This SDK is compatible
with Appwrite server version 1.8.x.**" reads "**This SDK is compatible with
Appwrite server version 1.9.x.**"; ensure consistency with the Version badge
(1.9.x) and the lib/appwrite.dart library header.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 68086c10-0304-46dd-873e-f37de377037c
📒 Files selected for processing (5)
CHANGELOG.mdREADME.mdlib/src/client_browser.dartlib/src/client_io.dartpubspec.yaml
✅ Files skipped from review due to trivial changes (1)
- pubspec.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- CHANGELOG.md
- lib/src/client_browser.dart
- lib/src/client_io.dart
ChiragAgg5k
changed the title
abnegate
approved these changes
Mar 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR contains updates to the Flutter SDK for version 23.0.0.