feat(offline_first): set policy for associations #569
Conversation
tshedor
left a comment
tshedor
left a comment
There was a problem hiding this comment.
@TillJohanndeiter Thank you for opening this PR. Could you please add tests for this too to the brick_offline_first package?
packages/brick_offline_first/lib/src/offline_first_repository.dart
Outdated
Show resolved
Hide resolved
tshedor
changed the title
![jit-ci[bot]](https://avatars.githubusercontent.com/in/142441?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ The following Jit checks failed to run:
- secret-detection
#jit_bypass_commit in this PR to bypass, Jit Admin privileges required.
More info in the Jit platform.
There was a problem hiding this comment.
❌ The following Jit checks failed to run:
- secret-detection
#jit_bypass_commit in this PR to bypass, Jit Admin privileges required.
More info in the Jit platform.
There was a problem hiding this comment.
❌ The following Jit checks failed to run:
- secret-detection
#jit_bypass_commit in this PR to bypass, Jit Admin privileges required.
More info in the Jit platform.
| get<TModel>( | ||
| query: query, | ||
| policy: policy, | ||
| associationPolicy: associationPolicy,).then( |
There was a problem hiding this comment.
This looks like it's missing a formatting check
| }) async { | ||
| return Horse( | ||
| name: data['name'] as String?, | ||
| mounties: await Future.wait<Mounty>( |
| this.name, | ||
| this.mounties = const <Mounty>[], | ||
| }); | ||
| final Owner? owner; |
There was a problem hiding this comment.
Instead of adding another model, why can't the existing Mounty be used?
| final localResults = await sqliteProvider.get<TModel>(repository: this); | ||
| final remoteResults = | ||
| await get<TModel>(query: query, policy: OfflineFirstGetPolicy.awaitRemote); | ||
| await get<TModel>(query: query, policy: OfflineFirstGetPolicy.awaitRemote, associationPolicy: associationPolicy); |
There was a problem hiding this comment.
Both OfflineFirstWithRestRepository and OfflineFirstWithGraphqlRepository need to be updated
| @override | ||
| Future<List<TModel>> get<TModel extends TRepositoryModel>({ | ||
| OfflineFirstGetPolicy policy = OfflineFirstGetPolicy.awaitRemoteWhenNoneExist, | ||
| OfflineFirstGetPolicy? associationPolicy, |
There was a problem hiding this comment.
The new parameter requires doc comments on this get method
| /// can be expensive for large datasets, making deserialization a significant hit when the result | ||
| /// is ignorable (e.g. eager loading). Defaults to `false`. | ||
| Future<List<TModel>> getBatched<TModel extends TRepositoryModel>({ | ||
| OfflineFirstGetPolicy? associationPolicy, |
There was a problem hiding this comment.
Same here - this will need documentation
2 participants
fixes #568
Add an option to set the OfflineFirstGetPolicy for associations. As described before i experienced performance issues if i use alwaysHydrate with Model that have (nested) associations. So i added here an optional parameter to specify the policy for associations.