-
Notifications
You must be signed in to change notification settings - Fork 368
feat(llc, persistence): add avgResponseTime
to User model
#2322
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
This commit introduces a new field to the `User` model: - `avgResponseTime`: An integer representing the average response time of the user in seconds. These field is now included in: - `User` and `OwnUser` model classes. - JSON serialization and deserialization for `User` and `OwnUser`. - The `UserEntity` in `stream_chat_persistence` and its corresponding database schema. - Relevant tests have been updated to include this new field.
WalkthroughA new nullable integer field, Changes
Sequence Diagram(s)sequenceDiagram
participant DB as Database
participant Mapper as UserMapper
participant Model as User/OwnUser Model
participant API as JSON API
API->>Model: Deserialize User JSON (includes avg_response_time)
Model->>Mapper: Map to UserEntity (avgResponseTime included)
Mapper->>DB: Store UserEntity (avgResponseTime in users table)
DB-->>Mapper: Retrieve UserEntity (avgResponseTime)
Mapper-->>Model: Map to User (avgResponseTime included)
Model-->>API: Serialize User (avgResponseTime excluded from toJson)
Estimated code review effort3 (~45 minutes) Suggested reviewers
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
avgResponseTime
to User modelavgResponseTime
to User model
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: 0
🔭 Outside diff range comments (1)
packages/stream_chat_persistence/lib/src/db/drift_chat_database.dart (1)
65-72
: Non-destructive migration for the new avgResponseTime columnInstead of dropping and recreating every table on any version change, add the new nullable column in place when upgrading to v23, and fall back to the full wipe only for breaking changes:
In packages/stream_chat_persistence/lib/src/db/drift_chat_database.dart, update the
onUpgrade
block around line 65:onUpgrade: (migrator, from, to) async { - if (from != to) { + // v22 ➔ v23: add nullable avg_response_time to Users without clearing data + if (from < 23) { + await migrator.addColumn(users, users.avgResponseTime); + } + + // fallback for any incompatible schema changes + if (from != to) { for (final table in allTables) { await migrator.deleteTable(table.actualTableName); } await migrator.createAll(); } },• Verify that your pubspec’s Drift version supports
migrator.addColumn
(Drift ≥1.4.0).
• Keep this pattern for future additive migrations before the destructive fallback.
🧹 Nitpick comments (2)
packages/stream_chat_persistence/lib/src/entity/users.dart (1)
37-39
: Consider indexing if you plan to filter/order by this field
avgResponseTime
is added as nullableIntColumn
, which is fine.
If analytics screens will frequently query or sort by this value, add
.indexed()
to improve lookup performance.No action needed otherwise.
packages/stream_chat/CHANGELOG.md (1)
3-6
: Minor nit – follow existing bullet styleElsewhere the changelog uses “-” bullets without emojis for added items.
Consider removing the "✅" prefix for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/stream_chat/CHANGELOG.md
(1 hunks)packages/stream_chat/lib/src/core/models/own_user.dart
(5 hunks)packages/stream_chat/lib/src/core/models/own_user.g.dart
(1 hunks)packages/stream_chat/lib/src/core/models/user.dart
(6 hunks)packages/stream_chat/lib/src/core/models/user.g.dart
(1 hunks)packages/stream_chat/test/fixtures/user.json
(1 hunks)packages/stream_chat/test/src/core/models/user_test.dart
(11 hunks)packages/stream_chat_persistence/CHANGELOG.md
(1 hunks)packages/stream_chat_persistence/lib/src/db/drift_chat_database.dart
(1 hunks)packages/stream_chat_persistence/lib/src/db/drift_chat_database.g.dart
(32 hunks)packages/stream_chat_persistence/lib/src/entity/users.dart
(1 hunks)packages/stream_chat_persistence/lib/src/mapper/user_mapper.dart
(2 hunks)packages/stream_chat_persistence/test/src/mapper/user_mapper_test.dart
(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build (ios)
- GitHub Check: test
- GitHub Check: build (android)
- GitHub Check: analyze
- GitHub Check: analyze_legacy_versions
- GitHub Check: stream_chat_flutter_core
- GitHub Check: stream_chat_localizations
- GitHub Check: stream_chat_flutter
- GitHub Check: stream_chat_persistence
🔇 Additional comments (28)
packages/stream_chat_persistence/lib/src/db/drift_chat_database.dart (1)
58-58
: Schema version bump acknowledgedBumping
schemaVersion
to 23 is expected for the new column addition.
No issues here.packages/stream_chat/test/fixtures/user.json (1)
16-18
: Fixture values look good – verify test assertions
avg_response_time
is represented in seconds as an integer (120).
Ensure updated tests assert this value to avoid silent mismatches.packages/stream_chat_persistence/CHANGELOG.md (1)
8-11
: Changelog entry OKEntry correctly documents the new field. Nothing to change.
packages/stream_chat_persistence/test/src/mapper/user_mapper_test.dart (1)
22-22
: LGTM! Comprehensive test coverage for the new field.The test updates properly cover bidirectional mapping of the
avgResponseTime
field betweenUserEntity
andUser
models. The test value of 120 seconds is reasonable and the assertions ensure correct mapping in both directions.Also applies to: 36-36, 51-51, 65-65
packages/stream_chat/lib/src/core/models/own_user.g.dart (1)
53-53
: LGTM! Proper JSON deserialization for the new field.The generated deserialization correctly maps the
avg_response_time
JSON key to theavgResponseTime
field with appropriate nullable integer handling.packages/stream_chat/lib/src/core/models/user.g.dart (2)
34-34
: LGTM! Proper JSON deserialization added.The deserialization correctly handles the
avgResponseTime
field from theavg_response_time
JSON key with appropriate nullable integer conversion.
37-42
: Verify if avgResponseTime should be included in JSON serialization.The
_$UserToJson
function doesn't includeavgResponseTime
. Confirm this is intentional (e.g., if it's a read-only server-computed field) or if it should be added to the serialization.packages/stream_chat_persistence/lib/src/mapper/user_mapper.dart (1)
18-18
: LGTM! Proper bidirectional mapping implemented.The
avgResponseTime
field is correctly mapped in both directions betweenUserEntity
andUser
models, maintaining consistency with the existing field mapping patterns.Also applies to: 35-35
packages/stream_chat/lib/src/core/models/own_user.dart (4)
35-35
: LGTM! Proper constructor integration.The
avgResponseTime
field is correctly added to the constructor and passed to the parentUser
class.
59-59
: LGTM! Complete field copying in factory method.The
fromUser
factory correctly copies theavgResponseTime
field from the sourceUser
instance.
86-86
: LGTM! Comprehensive copyWith implementation.The
avgResponseTime
parameter is properly added to thecopyWith
method signature and correctly handled in the null-coalescing assignment logic.Also applies to: 113-113
142-142
: LGTM! Consistent merge behavior.The
avgResponseTime
field is properly included in themerge
method, maintaining consistency with other field handling.packages/stream_chat/lib/src/core/models/user.dart (5)
50-50
: LGTM: Constructor parameter added correctly.The
avgResponseTime
parameter is properly added as a nullable integer field in the constructor.
78-78
: LGTM: Field added to topLevelFields for JSON deserialization.The 'avg_response_time' snake_case field name is correctly added to the topLevelFields array, enabling proper JSON deserialization.
148-150
: LGTM: Field declaration is well-documented and properly configured.The field is correctly declared as nullable integer with clear documentation and appropriate JSON annotation to exclude it from serialization while allowing deserialization.
180-180
: LGTM: copyWith method updated correctly.The
avgResponseTime
parameter is properly added to both the method signature and the constructor call within copyWith, maintaining consistency.Also applies to: 200-200
215-215
: LGTM: Field added to props for equality comparison.The
avgResponseTime
field is correctly included in the props getter, ensuring it participates in equality comparisons via Equatable.packages/stream_chat/test/src/core/models/user_test.dart (6)
21-22
: LGTM: Test constants defined appropriately.The test constants for
teamsRole
andavgResponseTime
are well-defined and will be used consistently across multiple test cases.
46-47
: LGTM: JSON parsing test validates new field.The test correctly verifies that
avgResponseTime
is properly parsed from JSON fixture data.
69-70
: LGTM: JSON serialization test updated correctly.The test verifies that
avgResponseTime
is included in the constructor but correctly notes that it's excluded from JSON serialization (as expected due to@JsonKey(includeToJson: false)
). TheteamsRole
field is properly serialized asteams_role
in snake_case.Also applies to: 82-82
101-102
: LGTM: copyWith tests comprehensively validate new field.The tests properly verify that
avgResponseTime
is correctly handled in both the default copyWith scenario (preserving existing values) and when explicitly updated with new values.Also applies to: 116-117, 132-133
218-219
: LGTM: Default value tests ensure proper null handling.The tests correctly verify that
avgResponseTime
defaults to null in both constructor and JSON parsing scenarios when not provided.Also applies to: 236-237
469-470
: LGTM: Helper function updated to support new fields.The
createTestUser
helper function is properly updated to accept and pass through the newteamsRole
andavgResponseTime
parameters, maintaining test utility consistency.Also applies to: 479-480
packages/stream_chat_persistence/lib/src/db/drift_chat_database.g.dart (5)
6188-6193
: Column schema definition looks correct.The
avgResponseTime
column is properly defined with appropriate nullability, data type (int for seconds), and naming conventions (snake_case for DB, camelCase for Dart).Also applies to: 6210-6210
6258-6263
: Validation logic follows the correct pattern.The validation for
avgResponseTime
properly checks for the database key and uses the appropriate validation metadata, consistent with other nullable fields.
6343-6344
: UserEntity integration is comprehensive and correct.The
avgResponseTime
field is properly integrated into all aspects of the UserEntity class:
- Clear documentation indicating it represents average response time in seconds
- Consistent handling in constructor, serialization, copyWith, equals, and hashCode
- Proper nullable field handling throughout
Also applies to: 6358-6359, 6385-6387, 6408-6408, 6425-6425, 6440-6440, 6452-6454, 6469-6471, 6488-6488, 6496-6496, 6510-6510
6524-6524
: UsersCompanion integration follows the correct companion pattern.The
avgResponseTime
field is properly integrated into the UsersCompanion class with:
- Correct Value wrapper usage for nullable fields
- Proper default Value.absent() initialization
- Consistent handling in copyWith and toColumns methods
Also applies to: 6537-6537, 6551-6551, 6566-6566, 6580-6580, 6596-6596, 6609-6609, 6646-6648, 6671-6671, 12422-12422, 12436-12436
12480-12482
: Query composer integration is correct.The
avgResponseTime
field is properly integrated into all query composers:
- Filter composer with appropriate ColumnFilters
- Ordering composer with ColumnOrderings
- Annotation composer with GeneratedColumn
- Table manager methods include the parameter consistently
Also applies to: 12527-12529, 12572-12573, 12612-12612, 12626-12626, 12640-12640, 12654-12654
Submit a pull request
Fixes: FLU-212
Description of the pull request
This commit introduces a new field to the
User
model:avgResponseTime
: An integer representing the average response time of the user in seconds.These field is now included in:
User
andOwnUser
model classes.User
andOwnUser
.UserEntity
instream_chat_persistence
and its corresponding database schema.Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation