feat: Flutter SDK update for version 20.3.1 #284
Conversation
|
Warning Rate limit exceeded@github-actions[bot] has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 15 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughThis pull request introduces version 20.3.1 with a focus on improving parameter handling and adding screenshot functionality. It adds three new enum definitions (Theme, Timezone, Output) to support avatar screenshot configuration options. The main refactoring converts API parameter constructions across multiple service files (account, avatars, databases, functions, storage, tables_db, teams) from unconditional inclusion to conditional inclusion of optional parameters only when non-null. A new public method Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
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 |
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: 3
🧹 Nitpick comments (1)
lib/services/account.dart (1)
823-841: Optional: Redundant null check in query building loop.With the conditional inclusion of optional parameters in the
paramsmap (lines 824-826, 1139-1141), the null check at lines 838 and 1153 (else if (value != null)) is now redundant. Values that are null will simply not be present in the map.You could simplify the query building to:
params.forEach((key, value) { if (value is List) { for (var item in value) { query.add( '${Uri.encodeComponent('$key[]')}=${Uri.encodeComponent(item)}'); } - } else if (value != null) { + } else { query.add('${Uri.encodeComponent(key)}=${Uri.encodeComponent(value)}'); } });Also applies to: 1138-1156
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
CHANGELOG.md(1 hunks)docs/examples/avatars/get-screenshot.md(1 hunks)lib/enums.dart(1 hunks)lib/services/account.dart(9 hunks)lib/services/avatars.dart(6 hunks)lib/services/databases.dart(12 hunks)lib/services/functions.dart(2 hunks)lib/services/storage.dart(6 hunks)lib/services/tables_db.dart(12 hunks)lib/services/teams.dart(4 hunks)lib/src/client_mixin.dart(2 hunks)lib/src/enums/output.dart(1 hunks)lib/src/enums/theme.dart(1 hunks)lib/src/enums/timezone.dart(1 hunks)test/services/avatars_test.dart(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/examples/avatars/get-screenshot.md
11-11: Bare URL used
(MD034, no-bare-urls)
39-39: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (16)
CHANGELOG.md (1)
3-5: Verify version consistency between PR title and CHANGELOG.The PR title references version 20.3.0, but the CHANGELOG documents version 20.3.1. Please ensure the version numbering is consistent across the PR title, CHANGELOG, and any other version references in the codebase (such as pubspec.yaml).
lib/src/client_mixin.dart (3)
20-22: LGTM! Proper null-safety for multipart values.The early return for null values prevents invalid multipart fields from being added to the request.
28-32: LGTM! Null filtering for list elements.Skipping null elements in lists when building multipart fields ensures only valid values are transmitted, preventing potential API errors.
44-56: LGTM! Comprehensive null-safe parameter filtering for GET requests.The implementation correctly:
- Filters out null values to reduce payload size and avoid sending unnecessary parameters
- Converts numeric types to strings (required for query parameters)
- Handles lists with the
key[]syntax (common REST API convention)This aligns well with the PR's goal of fixing null value handling.
test/services/avatars_test.dart (1)
149-160: LGTM! Appropriate test coverage for new method.The test follows the established pattern for avatar service methods and correctly validates the return type.
lib/services/teams.dart (3)
16-18: LGTM! Conditional parameter inclusion.The optional parameters (
queries,search,total) are correctly included only when non-null, reducing unnecessary data transmission and aligning with the PR's null-handling improvements.
41-41: LGTM! Proper handling of optional roles parameter.The
rolesparameter is correctly included only when provided.
162-167: LGTM! Appropriate conditional inclusion for optional parameters.All optional parameters (
userId,phone,url,name) are conditionally included, while the requiredrolesparameter is always present. This correctly implements the selective parameter inclusion pattern.lib/enums.dart (1)
10-12: LGTM! New enum exports.The three new enum types (Theme, Timezone, Output) are properly exposed via part directives, expanding the public API surface to support the new screenshot functionality.
lib/services/functions.dart (2)
17-18: LGTM! Conditional parameter inclusion.Optional parameters are correctly included only when non-null, consistent with the PR's null-handling improvements.
45-50: LGTM! Proper conditional inclusion of optional parameters.All optional parameters are correctly wrapped in null checks. Line 48's use of the bang operator (
method!.value) is safe because it's inside theif (method != null)condition.lib/src/enums/timezone.dart (1)
424-428: LGTM! Well-structured enum implementation.The Timezone enum is properly implemented with:
- A const constructor for compile-time constant support
- An immutable
valuefield for the underlying string representation- A
toJson()method for serializationThe comprehensive list of timezone identifiers provides excellent coverage for the screenshot functionality.
lib/services/account.dart (3)
36-41: LGTM! Clean refactoring to conditionally include optional parameters.The use of Dart's collection-if syntax to omit null values from API parameters is a clean improvement that prevents sending unnecessary null values to the backend.
Also applies to: 85-88, 140-143
564-567: Conditional parameter inclusion applied consistently.The refactoring correctly handles optional parameters across authentication and token-related endpoints.
Also applies to: 985-989, 1062-1066, 1099-1104
36-41: This review comment is incorrect. The changes intentionally fix a backend null parsing issue, not introduce one.The commit "fix null parsing issue in request body" explicitly addresses a backend problem where null values were causing parsing failures. The refactoring from sending nulls to omitting optional parameters is the standard solution and has been validated by:
- Consistent pattern applied across all optional parameters (
oldPassword,name,queries,total,success,failure,scopes,providerId,phrase,url)- Existing tests that call methods without optional parameters (e.g.,
updatePassword(password: '')withoutoldPassword)- Documentation confirming parameter optionality (e.g., "oldPassword is optional")
The backend already handles omitted optional fields correctly—that's why this fix resolves the parsing issue. No backend verification is needed; the change improves compatibility.
Likely an incorrect or invalid review comment.
lib/src/enums/output.dart (1)
1-17: LGTM! Well-structured enum for image output formats.The
Outputenum is cleanly implemented with comprehensive coverage of common image formats and proper JSON serialization support via thetoJson()method.
| Avatars avatars = Avatars(client); | ||
|
|
||
| // Downloading file | ||
| UInt8List bytes = await avatars.getScreenshot( |
There was a problem hiding this comment.
Fix type name typo.
Line 10 uses UInt8List but should be Uint8List (lowercase 'int').
Apply this diff:
-UInt8List bytes = await avatars.getScreenshot(
+Uint8List bytes = await avatars.getScreenshot(📝 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.
| UInt8List bytes = await avatars.getScreenshot( | |
| Uint8List bytes = await avatars.getScreenshot( |
🤖 Prompt for AI Agents
In docs/examples/avatars/get-screenshot.md around line 10, change the incorrect
type name "UInt8List" to the correct Dart type "Uint8List" (lowercase 'int');
update that occurrence (and any other occurrences in the file) so the example
compiles and matches Dart's Uint8List type name.
| viewportWidth: 1, // optional | ||
| viewportHeight: 1, // optional | ||
| scale: 0.1, // optional | ||
| theme: .light, // optional |
There was a problem hiding this comment.
Fix enum reference syntax.
The enum values are referenced with a leading dot without proper qualification. In Dart, enum values must be prefixed with the enum type or imported with a prefix.
Apply this diff to fix the enum references:
- theme: .light, // optional
+ theme: enums.Theme.light, // optional- timezone: .africaAbidjan, // optional
+ timezone: enums.Timezone.africaAbidjan, // optional- output: .jpg, // optional
+ output: enums.Output.jpg, // optionalThe same corrections apply to lines 44, 48, and 58 in the second example.
Also applies to: 20-20, 30-30
🤖 Prompt for AI Agents
In docs/examples/avatars/get-screenshot.md lines 16, 20, 30, 44, 48 and 58, enum
values are referenced with a leading dot (e.g., ".light") which is invalid in
Dart; update each enum reference to be fully qualified by its enum type (for
example change ".light" to "Theme.light" or the appropriate enum name
used/imported in the example) so all enum values are properly prefixed.
| enum Theme { | ||
| light(value: 'light'), | ||
| dark(value: 'dark'); |
There was a problem hiding this comment.
Rename this enum to avoid colliding with Flutter’s Theme widget.
Exposing an enum named Theme at the package root shadows package:flutter/material.dart’s Theme class. Any consuming app that imports both without prefixes will hit ambiguous/undefined symbol compile errors (e.g., Theme.of(context) stops resolving) once they upgrade to this release. Please pick a distinct name (for example ScreenshotTheme or AvatarThemeMode) and update the associated references before shipping.
🤖 Prompt for AI Agents
lib/src/enums/theme.dart lines 3-5: the enum named Theme conflicts with
Flutter’s Theme class; rename the enum (suggestion: ScreenshotTheme or
AvatarThemeMode) and update all internal references and exports accordingly.
Change the enum identifier and its variants (keeping the same values) in this
file, update any files that import or reference lib/src/enums/theme.dart to use
the new enum name, update any public re-exports or documentation, and run a
project-wide search/replace to fix usages and imports so no ambiguous Theme
symbol remains.
This PR contains updates to the Flutter SDK for version 20.3.1.