-
Notifications
You must be signed in to change notification settings - Fork 144
feat: Flutter SDK update for version 20.0.0 #274
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughUpdated SDK/example version to 20.0.0 (pubspec, README, x-sdk-version headers). Added enums ExecutionTrigger and ExecutionStatus and exposed them via lib/enums.dart; lib/models.dart imports enums. Execution model changed trigger and status from String to enums with value-based serialization/deserialization; tests updated. Document, Preferences, and Row factory constructors now prefer map["data"] when present, falling back to the full map. No other public API signature changes. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (5)
lib/src/enums/execution_status.dart (1)
3-14
: Provide a safe parser for API payloads.Right now every consumer has to hand-roll the string→enum mapping (usually with
values.firstWhere
), which will throw if the backend starts returning an unexpected value. Centralize the logic on the enum so model factories can fall back gracefully.enum ExecutionStatus { waiting(value: 'waiting'), processing(value: 'processing'), completed(value: 'completed'), failed(value: 'failed'); const ExecutionStatus({required this.value}); final String value; String toJson() => value; + + static ExecutionStatus? fromJson(String? value) { + if (value == null) return null; + for (final status in ExecutionStatus.values) { + if (status.value == value) { + return status; + } + } + return null; + } }lib/src/enums/execution_trigger.dart (2)
3-13
: Add parse helpers to centralize (de)serialization and improve resilienceProvide a static fromJson (and optionally tryParse) to avoid repeating firstWhere at call sites and to surface clearer errors for unknown values.
Apply this diff:
enum ExecutionTrigger { http(value: 'http'), schedule(value: 'schedule'), event(value: 'event'); const ExecutionTrigger({required this.value}); final String value; + static ExecutionTrigger fromJson(Object? value) { + switch (value) { + case 'http': + return ExecutionTrigger.http; + case 'schedule': + return ExecutionTrigger.schedule; + case 'event': + return ExecutionTrigger.event; + } + throw ArgumentError.value(value, 'value', 'Unknown ExecutionTrigger'); + } + + static ExecutionTrigger? tryParse(Object? value) { + switch (value) { + case 'http': + return ExecutionTrigger.http; + case 'schedule': + return ExecutionTrigger.schedule; + case 'event': + return ExecutionTrigger.event; + } + return null; + } + String toJson() => value; }
4-9
: Nit: prefer positional enum constructor for brevity (if aligned across enums)This removes named args noise on each enum value. Do this only if you update ExecutionStatus similarly for consistency.
-enum ExecutionTrigger { - http(value: 'http'), - schedule(value: 'schedule'), - event(value: 'event'); - - const ExecutionTrigger({required this.value}); +enum ExecutionTrigger { + http('http'), + schedule('schedule'), + event('event'); + + const ExecutionTrigger(this.value);lib/src/models/execution.dart (2)
24-27
: Public API change: trigger/status types switched from String to enumsThis is a breaking change for consumers accessing these fields as String. Confirm semver/migration (19.2.0) and update CHANGELOG/README. Consider adding convenience getters to ease migration (e.g., trigger.value/status.value).
Example (outside this diff hunk):
@Deprecated('Use trigger.value') String get triggerString => trigger.value; @Deprecated('Use status.value') String get statusString => status.value;
119-120
: Use enum.toJson() to centralize serializationKeeps symmetry with the enum and eases future changes.
- "trigger": trigger.value, - "status": status.value, + "trigger": trigger.toJson(), + "status": status.toJson(),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
README.md
(1 hunks)lib/enums.dart
(1 hunks)lib/models.dart
(1 hunks)lib/query.dart
(0 hunks)lib/src/client_browser.dart
(1 hunks)lib/src/client_io.dart
(1 hunks)lib/src/enums/execution_status.dart
(1 hunks)lib/src/enums/execution_trigger.dart
(1 hunks)lib/src/models/execution.dart
(3 hunks)pubspec.yaml
(1 hunks)test/query_test.dart
(0 hunks)test/src/models/execution_test.dart
(3 hunks)
💤 Files with no reviewable changes (2)
- test/query_test.dart
- lib/query.dart
🔇 Additional comments (7)
pubspec.yaml (1)
2-2
: Version metadata updated correctly.The published version bump matches the rest of the release notes and SDK headers.
lib/src/client_io.dart (1)
61-61
: IO client header version synced.The x-sdk-version header now reflects 19.2.0, keeping network calls aligned with the package version.
lib/src/client_browser.dart (1)
43-43
: Browser client header version synced.The browser transport now advertises the new SDK version, matching the rest of the release.
README.md (1)
24-24
: Docs now point to 19.2.0.The installation snippet reflects the latest published version, so newcomers will pull the correct release.
lib/models.dart (1)
4-5
: Alias import keeps model parts tidy.Bringing enums in under an alias keeps the generated part files explicit and avoids naming collisions.
lib/enums.dart (1)
13-14
: Enums surface expanded.Adding the execution trigger/status parts exposes the new enum types through the public enums library.
test/src/models/execution_test.dart (1)
2-48
: Tests exercise the enum-backed model.Pulling in the enums package and asserting against the new enum fields validates the serialization changes end-to-end.
trigger: enums.ExecutionTrigger.values.firstWhere( | ||
(e) => e.value == map['trigger'], | ||
), | ||
status: enums.ExecutionStatus.values.firstWhere( | ||
(e) => e.value == map['status'], | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle unknown enum values explicitly to avoid opaque StateError
firstWhere without orElse throws StateError on unknown/nullable values. Emit a clear error (or use the enum’s fromJson helper if added).
Apply this diff:
- trigger: enums.ExecutionTrigger.values.firstWhere(
- (e) => e.value == map['trigger'],
- ),
- status: enums.ExecutionStatus.values.firstWhere(
- (e) => e.value == map['status'],
- ),
+ trigger: enums.ExecutionTrigger.values.firstWhere(
+ (e) => e.value == map['trigger'],
+ orElse: () => throw ArgumentError.value(
+ map['trigger'],
+ 'trigger',
+ 'Unknown ExecutionTrigger',
+ ),
+ ),
+ status: enums.ExecutionStatus.values.firstWhere(
+ (e) => e.value == map['status'],
+ orElse: () => throw ArgumentError.value(
+ map['status'],
+ 'status',
+ 'Unknown ExecutionStatus',
+ ),
+ ),
If you add enums’ fromJson as suggested, prefer:
- trigger: enums.ExecutionTrigger.values.firstWhere( ... ),
- status: enums.ExecutionStatus.values.firstWhere( ... ),
+ trigger: enums.ExecutionTrigger.fromJson(map['trigger']),
+ status: enums.ExecutionStatus.fromJson(map['status']),
📝 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.
trigger: enums.ExecutionTrigger.values.firstWhere( | |
(e) => e.value == map['trigger'], | |
), | |
status: enums.ExecutionStatus.values.firstWhere( | |
(e) => e.value == map['status'], | |
), | |
trigger: enums.ExecutionTrigger.values.firstWhere( | |
(e) => e.value == map['trigger'], | |
orElse: () => throw ArgumentError.value( | |
map['trigger'], | |
'trigger', | |
'Unknown ExecutionTrigger', | |
), | |
), | |
status: enums.ExecutionStatus.values.firstWhere( | |
(e) => e.value == map['status'], | |
orElse: () => throw ArgumentError.value( | |
map['status'], | |
'status', | |
'Unknown ExecutionStatus', | |
), | |
), |
🤖 Prompt for AI Agents
In lib/src/models/execution.dart around lines 88 to 93, firstWhere is used
without orElse which will throw a vague StateError for unknown or null enum
values; change these lookups to either call a dedicated fromJson helper on the
enums or add an orElse that throws a clear, descriptive error (e.g.
FormatException or ArgumentError) indicating the invalid trigger/status and the
offending map value; ensure the error message includes the map key and value so
callers can diagnose malformed or unknown enum values.
lib/src/client_browser.dart
Outdated
'x-sdk-platform': 'client', | ||
'x-sdk-language': 'flutter', | ||
'x-sdk-version': '19.1.0', | ||
'x-sdk-version': '21.0.0', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skipped a version
This PR contains updates to the Flutter SDK for version 20.0.0.
Summary by CodeRabbit