GetDutchie / brick

An intuitive way to work with persistent data in Dart
https://getdutchie.github.io/brick/#/
361 stars 28 forks source link

bug(supabase-integration): Supabase query includes join even though there is no associationForeignKey defined #425

Closed devj3ns closed 2 months ago

devj3ns commented 2 months ago

I have a BuildingWindows model which has the following field:

@Supabase(foreignKey: null)
final List<WindowType>? windowTypes;

Because this is stored as a JSONB field in the Supabase DB (to simplify the sync), it does not specify a foreignKey because there is no remote WindowType table.

Even though the foreignKey is null, the REST Request includes a join inside the select clause, which should not be the case in my opinion. Because of the invalid join, the Postgrest API throws the following exception:

PostgrestException(
     message: "failed to parse select parameter (id,project_id,window_types(id,glazing,frame_material,manufacturing_year,locations),notes)", 
     code: PGRST100, 
     details: unexpected "(" expecting ":" or field name (* or [a..z0..9_$]), 
     hint: null)

Moreover, I noticed, that the associationForeignKey field inside the adapter is associationForeignKey: 'null' instead of associationForeignKey: null when no associationForeignKey was provided in the model field @Supabase annotation.

tshedor commented 2 months ago

@devj3ns Can you provide the class definition for WindowType? Is it an OfflineFirstWithSupabaseModel? Is it a serdes?

I'm interested in knowing how the adapter has interpreted this class. It should only make those associations if the builder recognizes the declared type to be a sibling to the model you're using (assuming the model where windowTypes exists to be an OfflineFirstWithSupabaseModel).

Good catch on the 'null' thing, will fix in #426 .

devj3ns commented 2 months ago

@tshedor, WindowType is a model:

@ConnectOfflineFirstWithSupabase()
class WindowType extends OfflineFirstWithSupabaseModel {
  WindowType({
    required this.id,
    required this.glazing,
    required this.frameMaterial,
    required this.manufacturingYear,
    required this.locations,
  });

  @Sqlite(unique: true, index: true)
  @Supabase(unique: true)
  final UUID id;

  @Sqlite(enumAsString: true, nullable: true)
  @Supabase(enumAsString: true, nullable: true)
  final WindowGlazing? glazing;

  @Sqlite(enumAsString: true, nullable: true)
  @Supabase(enumAsString: true, nullable: true)
  final WindowFrameMaterial? frameMaterial;

  final int? manufacturingYear;

  final String? locations;
}
tshedor commented 2 months ago

If it's a model, shouldn't it have a database table equivalent? Otherwise it should be a serdes if it's only JSONB in Supabase

devj3ns commented 2 months ago

Yes, you are totally right. I will change that.