Skip to content
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

Migration not created for Sqlite Database when extending Model after upgrade to 3.0 #343

Closed
iliraga opened this issue May 25, 2023 · 4 comments
Assignees

Comments

@iliraga
Copy link

iliraga commented May 25, 2023

Because of the whole setup it would be a little hard to provide a working sample of this, so I try to explain the issue as best as possible.

There is an entity, in our case user.model.dart which has a list of various annotations:

@DataClass()
@JsonSerializable()
@ConnectOfflineFirstWithRest(
  restConfig: RestSerializable(
    requestTransformer: UserSyncEndpointHelper.new,
    fieldRename: FieldRename.none,
  ),
)

What I did now, is to extend that particular model class with a new property:

final bool? isNew (and extended the constructor as well).

Before doing the upgrade to brick 3, it usually was enough to run the dart run build_runner build command. This generated the actual user_adapter.g.dart and user.model.g.dart file, and in addition a database migration including the changed database schema and the actual change in the table (an InsertColumn-command basically).

In my understanding the first two files were generated by the rest-subpackage, whereas the latter one was generated by the sqlite subpackage.

The first part works like usually, as proof here the git status-result. Even the changes itself within those files look reasonable:

modified:   lib/brick/adapters/user_adapter.g.dart
modified:   lib/user_management/user/models/user.model.dart
modified:   lib/user_management/user/models/user.model.g.dart
modified:   lib/user_management/user/user_service.dart
detailed GIT DIFF showing all changes autogenerated
diff --git a/packages/core/lib/user_management/user/models/user.model.dart b/packages/core/lib/user_management/user/models/user.model.dart
index 36a6bf0..28acdc1 100644
--- a/packages/core/lib/user_management/user/models/user.model.dart
+++ b/packages/core/lib/user_management/user/models/user.model.dart
@@ -21,13 +21,13 @@ class User extends SyncModel with _$User implements EngineModel {
     this.country,
     this.gender,
     this.id = '',
+    this.isNew,
     this.referral,
     this.city,
     this.nationality,
     this.postalCode,
     this.streetAddress,
     this.profilePicUrl,
-    this.isNew,
     this.contestPasses = const [],
     this.gig,
     this.connectors = const [],
@@ -82,8 +82,6 @@ class User extends SyncModel with _$User implements EngineModel {
   /// Url for profile pic
   final String? profilePicUrl;
 
-  /// Specifies whether the user is new. In those cases the user can be granted
-  /// all kind of subscriptions without checking them.
   final bool? isNew;
 
   /// Referral
diff --git a/packages/core/lib/user_management/user/models/user.model.g.dart b/packages/core/lib/user_management/user/models/user.model.g.dart
index e0da2f2..d98e226 100644
--- a/packages/core/lib/user_management/user/models/user.model.g.dart
+++ b/packages/core/lib/user_management/user/models/user.model.g.dart
@@ -105,13 +105,13 @@ mixin _$User {
       country: country ?? _self.country,
       gender: gender ?? _self.gender,
       id: id ?? _self.id,
+      isNew: isNew ?? _self.isNew,
       referral: referral ?? _self.referral,
       city: city ?? _self.city,
       nationality: nationality ?? _self.nationality,
       postalCode: postalCode ?? _self.postalCode,
       streetAddress: streetAddress ?? _self.streetAddress,
       profilePicUrl: profilePicUrl ?? _self.profilePicUrl,
-      isNew: isNew ?? _self.isNew,
       contestPasses: contestPasses ?? _self.contestPasses,
       gig: gig ?? _self.gig,
       connectors: connectors ?? _self.connectors,
@@ -190,13 +190,13 @@ class UserChanges {
         country: country,
         gender: gender,
         id: id,
+        isNew: isNew,
         referral: referral,
         city: city,
         nationality: nationality,
         postalCode: postalCode,
         streetAddress: streetAddress,
         profilePicUrl: profilePicUrl,
-        isNew: isNew,
         contestPasses: contestPasses,
         gig: gig,
         connectors: connectors,
@@ -218,6 +218,7 @@ User _$UserFromJson(Map<String, dynamic> json) => User(
       country: json['country'] as String?,
       gender: json['gender'] as String?,
       id: json['id'] as String? ?? '',
+      isNew: json['isNew'] as bool?,
       referral: json['referral'] == null
           ? null
           : ReferrerView.fromJson(json['referral'] as Map<String, dynamic>),
@@ -226,7 +227,6 @@ User _$UserFromJson(Map<String, dynamic> json) => User(
       postalCode: json['postalCode'] as String?,
       streetAddress: json['streetAddress'] as String?,
       profilePicUrl: json['profilePicUrl'] as String?,
-      isNew: json['isNew'] as bool?,
       contestPasses: json['contestPasses'] == null
           ? const []
           : ContestPassView.contestPassesFromJson(
diff --git a/packages/core/lib/user_management/user/user_service.dart b/packages/core/lib/user_management/user/user_service.dart
index 9012088..efa5129 100644
--- a/packages/core/lib/user_management/user/user_service.dart
+++ b/packages/core/lib/user_management/user/user_service.dart
@@ -98,7 +98,9 @@ class UserService extends EngineModelService<User> {
       user = await _syncService.getLatest();
     }
 
-    _userLoaded$.add(user?.isValid() ?? false);
+    _userLoaded$.add(
+      user?.isValid() ?? false,
+    );
 
     model(user);
   }

but there is no migration created in this case, hence the actual synchronisation fails (storing it in the database). The retrieval from the remote-source works (e.g. retrieval from the rest service).

In the migration to 3-guide I didn't find anything related there. All the steps like importing different packages, or converting the RestSerializable endpoint-property with an actual requestTransformer seems to have worked (although that part is working, as explained before). Am I missing something here?

In addition the related packages and it versions:

  brick_offline_first: ^3.0.3
  brick_offline_first_with_rest: ^3.0.1
  brick_rest: ^3.0.2
  brick_sqlite: ^3.0.1
  mek_data_class: ^1.1.0

dev_dependencies:
  brick_offline_first_with_rest_build: ^3.0.1
  mek_data_class_generator: ^1.2.0
  json_serializable

flutter doctor shows the following:

Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel stable, 3.10.2, on macOS 13.2.1 22D68 darwin-x64, locale en-CH)
[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.1)
[✓] Xcode - develop for iOS and macOS (Xcode 14.3)
[✓] Chrome - develop for the web
[✓] Android Studio (version 2020.3)
[✓] VS Code (version 1.78.2)
[✓] Connected device (3 available)
[✓] Network resources

• No issues found!

If relevant, the build.yaml:

targets:
  $default:
    builders:
      mek_data_class_generator|data_class:
        enabled: true
        options:
          page_width: 80
          comparable: true
          stringify: true
          copyable: true
          changeable: true
          changes_visible: true
@tshedor
Copy link
Collaborator

tshedor commented Jun 20, 2023

@iliraga I'm so sorry for the delay; I thought I'd responded much earlier. Very sorry. A couple clarifying questions:

  1. To confirm, modified: lib/brick/adapters/user_adapter.g.dart is correctly modified to include the new field in toSqlite?
  2. Your git diff shows isNew being moved but not strictly inserted. The SchemaDifference generator wouldn't detect this as a column insertion. Does schema.g.dart definitely not include the is_new?
    +    this.isNew,
    this.referral,
    this.city,
    this.nationality,
    this.postalCode,
    this.streetAddress,
    this.profilePicUrl,
    -    this.isNew,
    
  3. Is it just this field that isn't being generated as a migration? Or if you add, say, another new field like isNewAgain to the User model, that also isn't generated? Additionally, could you try adding another field to a different model? I'd like to narrow down if it's this field or this model or this version of Brick.

Again, sorry for not getting to you sooner.

@tshedor
Copy link
Collaborator

tshedor commented Jun 26, 2023

@iliraga Could you please upgrade to brick_offline_first_with_rest_build 3.1.0? I think it should solve your problem.

@tshedor
Copy link
Collaborator

tshedor commented Jun 30, 2023

@iliraga heads up, I'm going to close this ticket next week. Hoping this fix worked for you

@tshedor tshedor closed this as completed Jul 3, 2023
@iliraga
Copy link
Author

iliraga commented Jul 20, 2023

Hi @tshedor sorry somehow I missed your comments as well. I did the manual fix a while ago by manually generating the migration (very carefully). Unfortunately this is so long ago, that I can't properly reproduce the same situation again, so let's leave it at that for now. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants