search

aws-amplify / amplify-flutter

A declarative library with an easy-to-use interface for building Flutter applications on AWS.
https://docs.amplify.aws
Apache License 2.0
1.31k stars 243 forks source link

Generated Models relationship ID fields missing #1564

Open Lorenzohidalgo opened 2 years ago

Lorenzohidalgo commented 2 years ago

Description

The automatically generated Data Models are missing the relationship IDs.

schema.graphql example:

type NPartner @model @auth(rules: [{allow: private}]) {
  id: ID!
  pid: ID
  avatarKey: String
  name: String!
  description: AWSJSON!
  webURL: AWSJSON!
  email: String!
  packs: [NTokenPack] @hasMany(indexName: "tokenPackByPartnerPID", fields: ["id"])
}

type NTokenPack @model @auth(rules: [{allow: private}]) {
  id: ID!
  pid: ID
  name: AWSJSON!
  description: AWSJSON!
  partner: NPartner @belongsTo(fields: ["partnerPID"])
  partnerPID: ID! @index(name: "tokenPackByPartnerPID", queryField: "tokenPackByPartnerPID")
  categories: [TokenCategory] @hasMany(indexName: "tokenCategoryByPackPID", fields: ["id"])
}

As defined by the schema, the NTokenPack model has the required field partnerPID.

The generated model has only the following variables available:

  static const classType = const _NTokenPackModelType();
  final String id;
  final String pid;
  final String avatarKey;
  final String name;
  final String description;
  final NPartner partner;
  final List<TokenCategory> categories;
  final TemporalDateTime createdAt;
  final TemporalDateTime updatedAt;

  const NTokenPack._internal(
      {@required this.id,
      this.pid,
      this.avatarKey,
      @required this.name,
      @required this.description,
      this.partner,
      this.categories,
      this.createdAt,
      this.updatedAt});

This presents the following issues:

  1. The variable partnerPID is not present.
  2. The variable partner is present but not required.
  3. Therefore there is no "reliable" way of getting the id of the partner

Since it's not a one-to-one relationship, I won't be needing to query for the related partner every time. I would like to be able to access the partnerPID variable without having to declare/query for the whole partner model each time.

Categories

Steps to Reproduce

  1. Copy paste the example schema
  2. Deploy it
  3. Generate the Data Models

Screenshots

No response

Platforms

Android Device/Emulator API Level

No response

Environment

Doctor summary (to see all details, run flutter doctor -v):
[√] Flutter (Channel stable, 2.10.3, on Microsoft Windows [Versión 10.0.19044.1645], locale es-ES)
[√] Android toolchain - develop for Android devices (Android SDK version 31.0.0)
[√] Chrome - develop for the web
[!] Visual Studio - develop for Windows (Visual Studio Build Tools 2017 15.9.30)
    X Visual Studio 2019 or later is required.
      Download at https://visualstudio.microsoft.com/downloads/.
      Please install the "Desktop development with C++" workload, including all of its default components
[√] Android Studio (version 4.1)
[√] VS Code, 64-bit edition (version 1.66.2)
[√] Connected device (3 available)
[√] HTTP Host Availability

Dependencies

Dart SDK 2.16.1
Flutter SDK 2.10.3
bonusbank_app 0.1.5+43

dependencies:
- amplify_analytics_pinpoint 0.4.5 [amplify_analytics_plugin_interface amplify_analytics_pinpoint_android amplify_analytics_pinpoint_ios amplify_core flutter plugin_platform_interface]
- amplify_api 0.4.5 [amplify_api_plugin_interface amplify_core collection flutter meta plugin_platform_interface]
- amplify_auth_cognito 0.4.5 [flutter amplify_auth_plugin_interface amplify_core amplify_auth_cognito_android amplify_auth_cognito_ios collection plugin_platform_interface]
- amplify_core 0.4.5 [flutter plugin_platform_interface collection date_time_format meta uuid]
- amplify_datastore 0.4.5 [flutter amplify_datastore_plugin_interface amplify_core plugin_platform_interface meta collection async]
- amplify_flutter 0.4.5 [amplify_analytics_plugin_interface amplify_api_plugin_interface amplify_auth_plugin_interface amplify_core amplify_datastore_plugin_interface amplify_storage_plugin_interface collection flutter json_annotation meta plugin_platform_interface]
- amplify_storage_s3 0.4.5 [flutter amplify_storage_plugin_interface plugin_platform_interface amplify_storage_s3_android amplify_storage_s3_ios amplify_core]
- back_button_interceptor 5.0.2 [collection flutter]
- cached_network_image 3.2.0 [flutter flutter_cache_manager octo_image cached_network_image_platform_interface cached_network_image_web]
- carousel_slider 4.0.0 [flutter]
- date_util 0.1.4
- firebase_analytics 9.1.5 [firebase_analytics_platform_interface firebase_analytics_web firebase_core firebase_core_platform_interface flutter]
- firebase_core 1.14.1 [firebase_core_platform_interface firebase_core_web flutter meta]
- firebase_messaging 11.2.13 [firebase_core firebase_core_platform_interface firebase_messaging_platform_interface firebase_messaging_web flutter meta]
- firebase_performance 0.8.0+9 [firebase_core firebase_core_platform_interface firebase_performance_platform_interface firebase_performance_web flutter]
- firebase_remote_config 2.0.4 [firebase_core firebase_core_platform_interface firebase_remote_config_platform_interface firebase_remote_config_web flutter]
- flutter 0.0.0 [characters collection material_color_utilities meta typed_data vector_math sky_engine]
- flutter_bloc 7.3.3 [flutter bloc provider]
- flutter_local_notifications 9.3.2 [clock flutter flutter_local_notifications_linux flutter_local_notifications_platform_interface timezone]
- flutter_localizations 0.0.0 [flutter intl characters clock collection material_color_utilities meta path typed_data vector_math]
- flutter_svg 1.0.3 [flutter meta path_drawing vector_math xml]
- getwidget 2.0.4 [flutter]
- google_fonts 2.3.1 [flutter http path_provider crypto]
- image_cropper 1.4.1 [flutter]
- image_picker 0.8.4+9 [flutter flutter_plugin_android_lifecycle image_picker_for_web image_picker_platform_interface]
- in_app_review 2.0.4 [flutter in_app_review_platform_interface]
- intl 0.17.0 [clock path]
- jiffy 5.0.0 [intl]
- local_auth 1.1.11 [flutter flutter_plugin_android_lifecycle intl platform]
- new_version 0.3.0 [flutter package_info_plus http html url_launcher collection]
- overlay_support 1.2.1 [flutter async]
- package_info_plus 1.3.1 [flutter package_info_plus_platform_interface package_info_plus_linux package_info_plus_macos package_info_plus_windows package_info_plus_web]
- path_provider 2.0.9 [flutter path_provider_android path_provider_ios path_provider_linux path_provider_macos path_provider_platform_interface path_provider_windows]
- percent_indicator 4.0.0 [flutter]
- responsive_framework 0.1.7 [flutter collection]
- rive 0.8.4 [collection flutter graphs http meta]
- sentry_flutter 6.4.0 [flutter flutter_web_plugins sentry package_info_plus meta]
- shared_preferences 2.0.13 [flutter shared_preferences_android shared_preferences_ios shared_preferences_linux shared_preferences_macos shared_preferences_platform_interface shared_preferences_web shared_preferences_windows]
- syncfusion_flutter_charts 20.1.48 [flutter intl vector_math syncfusion_flutter_core]
- url_launcher 6.0.20 [flutter url_launcher_android url_launcher_ios url_launcher_linux url_launcher_macos url_launcher_platform_interface url_launcher_web url_launcher_windows]

Device

N/A

OS

N/A

CLI Version

8.0.2

Additional Context

No response

Jordan-Nelson commented 2 years ago

Hi @Lorenzohidalgo - Can you include the portion of your schema that defines TokenCategory?

The variable partner is present but not required

It looks like it is not required in the schema. If you want to make it required, you can update:

partner: NPartner @belongsTo(fields: ["partnerPID"])

to:

partner: NPartner! @belongsTo(fields: ["partnerPID"])
Lorenzohidalgo commented 2 years ago

Hi @Jordan-Nelson,

the schema for TokenCategory is:

type TokenCategory @model @auth(rules: [{allow: private}]) {
  id: ID!
  pid: ID
  partnerPID: ID! @index(name: "tokenCategoryByPartnerPID", queryField: "tokenCategoryByPartnerPID")
  tokenPackPID: ID! @index(name: "tokenCategoryByPackPID", queryField: "tokenCategoryByPackPID")
  tokens: [NToken] @hasMany(indexName: "ntokenByCategoryPID", fields: ["id"])
  dbIcon: AWSJSON!
  name: AWSJSON!
  description: AWSJSON!
  action: AWSJSON
  valueType: ValueTypeEnum!
  color: String!
}

The issue here is that partnerPID is not present. I want partner to be nullable. Sorry if I didn't express it correctly.

Lorenzohidalgo commented 2 years ago

What I meant by:

  1. The variable partner is present but not required.
  2. Therefore there is no "reliable" way of getting the id of the partner

Is that with the currently generated model, I could end up creating an instance of NTokenPack with the missing partnerPID (which is required). I'm also unable to access the partnerPID variable if I don't query for the partner also.

The "fix" or expected behavior would be to include the partnerPID variable in the generated model. This is also the behaviour if I remove the @belongsTo(fields: ["partnerPID"]) statement.

fjnoyp commented 2 years ago

Hi @Lorenzohidalgo

  1. You need to mark partner as required in the schema.graphql if you want it to be required in the generated model. You've already done that for other fields with the !.
  2. Yes as the field is not required it is nullable so you can create a NTokenPack without a partner.

Once you've made the partner field required and you've created a NTokenPack with a partner, try querying for your NTokenPack. In the resulting model returned, the partner field should be filled. The fields of this partner field will be null but its id field will not be. That is the equivalent of the partnerPID variable you are trying to access.

Lorenzohidalgo commented 2 years ago

Hi @fjnoyp,

This doesn't solve the issue. I don't want to query for the partner every single time. I just need the generated model to have the partnerPID variable available.

Generating the model without the @belongsTo relationship does generate the model with the partnerPID variable.

Adding partner: NPartner @belongsTo(fields: ["partnerPID"]) to the model should only add the partner variable and not remove the partnerPID one.

Jordan-Nelson commented 2 years ago

Hi @Lorenzohidalgo - Sorry for the delayed response.

I am not sure why the @belongsTo directive removes the ID field. I can see if this is intentional, or possibly an oversight. As a work around, you may be able to make some updates to your schema to use the @hasOne directive. I believe the ID field would still be present with a schema such as the one below.

You can find more info about the hasOne directive in the docs here: https://docs.amplify.aws/cli/graphql/data-modeling/#has-one-relationship

type Parent @model {
  id: ID!
  name: String
  childID: ID
  child: Child @hasOne(fields: ["childID"])
}

type Child @model {
  id: ID!
  name: String
}
Lorenzohidalgo commented 2 years ago

Hi @Jordan-Nelson,

The Docs state:

The @hasOne and @hasMany directives do not support referencing a model which then references the initial model via @hasOne or @hasMany if DataStore is enabled.

So I understand that the proposed workaround won't work.

Jordan-Nelson commented 2 years ago

@Lorenzohidalgo - Ah, I missed that this was a hasMany relationship. Apologies.

Jordan-Nelson commented 2 years ago

@Lorenzohidalgo - I had a missed a few things in your original issue description. Some notes below.

The variable partnerPID is not present The variable partner is present but not required

If you have fetched nTokenPack via Amplify.DataStore.query(), you should be able to access this via nTokenPack.partner!.id. The parent (partner) should always be eagerly loaded in a hasMany <-> belongsTo relationship, meaning that if nTokenPack is fetched with DataStore, it should include partner.

Can you confirm that partner is present when nTokenPack is fetched via datastore operations, and that the partner ID can be accessed via nTokenPack.partner!.id?

In older versions of DataStore, the associated model was not automatically loaded. partner would have had to be fetched manually. I believe this is the reason why partner is present but not required in the generated model. Having to cast this is definitely a sub-optimal developer experience though.

We have an issue open to track developer experience improvements around associated models (https://github.com/aws-amplify/amplify-flutter/issues/1449). The improvements being tracked by that issue will likely cover this. The generated model could potentially be updated to make associated model non-null until as a short term fix until 1449 is picked up. I think we can keep this issue open as a separate request to look into a short term to improvement.

Jordan-Nelson commented 2 years ago

The parent (partner) should always be eagerly loaded in a hasMany <-> belongsTo relationship

I wanted to add a clarification here, the parent will always be loaded, assuming the parent exists. So, unless you are sure the parent exists, I would access the parent ID via nTokenPack.partner?.id.

Jordan-Nelson commented 2 years ago

If the parent ID were added to the model, it would have to be optional since the relationship is not required in the schema. So I think nTokenPack.partner?.id would be a sufficient replacement for what you were originally looking for - nTokenPack.partnerID.

I would have to confirm, but I think partner would be non-null if the schema was updated to be required, but it sounds like you do not want it to be required.

Does using nTokenPack.partner?.id work for your use case?

Jordan-Nelson commented 2 years ago

I see you are using API, not DataStore. I had thought you were using DataStore. The schema and generated models are used for both. Most of what I said above was specific to DataStore.

Jordan-Nelson commented 2 years ago

Sorry for the confusion. I was having a hard time understanding what you are trying to do since I thought this was for DataStore.

I'm going to mark this as a feature request since there isn't currently a way access the nested model ID without creating the full nested model as you had stated.

TheWhiteShade commented 2 years ago

Jordan-Nelson.Still waiting for your update.

Jordan-Nelson commented 2 years ago

Hello @TheWhiteShade - I don't have an update on this feature request at the moment.

cjsilva-umich commented 11 months ago

Anyone have any workarounds for this? Why is the id field missing?