Morph one

[ryandialpad]

Type of PR:

• Bugfix
• Feature
• Refactor
• Code style update
• Build-related changes
• Test
• Documentation
• Other, please describe:

Breaking changes:

• No
• Yes

Details

Adding Morph One Polymorphic Relation & accompanying feature tests. Rest of description is TBD.

Hoping to get some early feedback

Docs to be added later

[ryandialpad]

You will see the class definitions and data duplicated across the feature tests. I have already discussed this and since many of the feature tests have slight differences in their class definitions and data, it is better to explicitly define these for each file / test (following the established testing patterns).

[ryandialpad]

To give some context on testing requirements for a relation:

Each relation has a set of 4 feature test suites which follow the same structure:

my_relation_retrieve
my_relation_save
my_relation_save_uid
my_relation_save_custom_key

[ryandialpad]

Following the API already established in Vuex ORM.

See Here & Docs: https://github.com/vuex-orm/vuex-orm/blob/master/src/attributes/relations/MorphOne.ts

I personally would be open to renaming these two fields to make them a little more explicit. I plan on also discussing this with the author but was curious as to whether or not either of you had any thoughts?

[superfreddialpad]

certainly bring it up with Kia. I agree with the sentiment, but IMO following the established api is more important.

[ryandialpad]

I have discussed this with the author and we believe this is the best approach to fetch the name of the table / entity

[ryandialpad]

Quick explanation of the where constraints on the relation:

The type field should match the parent's entity and the id field should match the parent's local key

[superfreddialpad]

this is likely a question borne of typescript ignorance, but why do these need to be cast as any? IIRC the first argument of where/whereIn should work the the actual types being used here.

Is tsc complaining that this.type/this.id could be null/undef? If that's the case, I would think the solution would be to define these props are non-null (i.e. protected type!: string) but again, this may be my ignorance of typescript speaking.

[ryandialpad]

Had a closer look, they don't. I was following the previous usages of query.whereIn and query.where in the codebase 🤦

[ryandialpad]

Great catch!

[ryandialpad]

Morph One Relation

I am a hands on learner so I figured that an example might help give a clarity as to what is going on in the Morph One relation.

Example

class Image extends Model {
  static entity = 'images'

  @Attr() id!: number
  @Str('') url!: string
  @Attr() imageable_id!: number
  @Attr() imageable_type!: string
}

class User extends Model {
  static entity = 'users'

  @Attr() id!: number
  @Str('') name!: string

  @MorphOne(() => Image, 'imageable_id', 'imageable_type')
  image!: Image | null
}

class Post extends Model {
  static entity = 'posts'

  @Attr() id!: number
  @Str('') title!: string
  @MorphOne(() => Image, 'imageable_id', 'imageable_type')
  image!: Image | null
}

Methods

getRelateds

Returns related models. The method is used by the database to register any related models which have yet to be registered.

In our example it would return [ Image {} ]

attach

Attaches the relational key to the related model

In our example it would set the following:

Image.imageable_id = User.id
Image.imageable_type = User.entity

addEagerConstraints, buildDictionary, match

These methods are used by the Query object to eagerly fetch related data. The Query object will apply the eager constraints prior to matching the related data.

Example Usage: store.$repo(Model).with('related-entity')

addEagerConstraints

Applies constraints to fetch related data.

In our example we would have the following constraints applied:

type = 'users'
[ User.id ].includes(imageable_id)

buildDictionary

Builds a dictionary of related data using the parent's id.

Built Dictionary Example:

{
  // the key will match the imageable_id for our example
  '1': [
    Image {
      id: 1,
      url: '/profile.jpg',
      imageable_id: 1,
      imageable_type: 'users'
    }
  ]
}

match

Builds a dictionary of related data then sets a relation for each parent model.

Example of the updated collection of models with their related data:

[
  User {
    id: 1,
    name: 'John Doe',
    image: Image {
      id: 1,
      url: '/profile.jpg',
      imageable_id: 1,
      imageable_type: 'users'
    }
  }
]

make

Used to create a related model

Example Usage:

store.$repo(User).make({
  id: 1,
  name: 'John Doe',
  ...
  image: {
    ...
    url: '/profile.jpg'
  }
})

define

Defines the Normalizr schema which we build on top of.

[superfreddialpad]

This looks good to me, but that has to be taken with a grain of salt, since I'm still not that familiar with the project (or indeed with ORM implementation best practices). That said, I think I'd be comfortable submitting this for upstream review.

[superfreddialpad]

certainly bring it up with Kia. I agree with the sentiment, but IMO following the established api is more important.

[superfreddialpad]

this is likely a question borne of typescript ignorance, but why do these need to be cast as any? IIRC the first argument of where/whereIn should work the the actual types being used here.

Is tsc complaining that this.type/this.id could be null/undef? If that's the case, I would think the solution would be to define these props are non-null (i.e. protected type!: string) but again, this may be my ignorance of typescript speaking.