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

feat(objection): add model relationship decorators #2078

Merged

Conversation

rossyman
Copy link
Contributor

@rossyman rossyman commented Sep 3, 2022

Information

Type Breaking change
Feature No

First time contributor here 😄 If there are any striking concerns about this PR then please let me know, as I'm happy to work on it. I thought that it'd be useful for developers to use decorators to describe relationships via Objection.

This PR hopefully honours the way that Objection's Graph and Relationship Querying APIs work -- whilst also ensuring the consumer's domain model is a bit more strict, by actually defining related properties on an entity as optional, instead of only loosely defining them via relationMappings.

The sensible defaults these decorators provide for joining keys are based on the use of @IdColumn and @Entity. The defaults supplied are based on the Objection relationship documentation and follow the same nomenclature with the ability for a user to override the keys / table path in the decorator factory params.


Usage example

/**
 * All work in a similar manner:
 * - @HasMany, @HasOne, @HasOneThroughRelation, @ManyToMany, @RelatesTo
 */
import { Entity, IdColumn, BelongsToOne } from '@tsed/objection';

@Entity("user")
class User extends Model {
  @IdColumn()
  id!: string;
}

@Entity("movie")
class Movie extends Model {

  @IdColumn()
  id!: string;

  ownerId!: string;

  @BelongsToOne()
  owner?: User;
}

// Retrieve the related user
const owner = await Movie.relatedQuery('owner').for(1);

// Retrieve the movie with their owner
const movie = await Movie.query().withGraphFetched('owner');

Todos

  • Tests
  • Coverage
  • Example
  • Documentation

@rossyman
Copy link
Contributor Author

rossyman commented Sep 3, 2022

Some extra thought I had on this PR:

  • It would be nice if we could infer the modelClass from the type defined on the property (Not even sure if this is possible)
  • It would be nice if we could make the join options supplied to this decorator less nested

@rossyman rossyman force-pushed the feat-objection-relation-decorators branch from d334351 to e97a55a Compare September 4, 2022 14:28
@rossyman
Copy link
Contributor Author

rossyman commented Sep 4, 2022

Last thought I had on this, was that:

  • It would be nice if we could set a sensible default for relationship joins

@rossyman rossyman force-pushed the feat-objection-relation-decorators branch from e97a55a to c72d935 Compare September 5, 2022 01:08
@Romakita
Copy link
Collaborator

Romakita commented Sep 5, 2022

Hello @rossyman,

It seems to be ok for me :) Good PR.

The documentation over the new decorators is missing. Don't forget to add it in /docs/tutorials/objection.md ;)

See you
Romain

@rossyman rossyman force-pushed the feat-objection-relation-decorators branch from c72d935 to e650b24 Compare September 5, 2022 11:14
@rossyman
Copy link
Contributor Author

rossyman commented Sep 5, 2022

Updated the documentation and fixed the build/test phase -- One point, is that I'm not sure what I need to do for viewpress to find these new decorators, as at the moment, they're not showing up in the <ApiList> component in the markdown.

@Romakita
Copy link
Collaborator

Romakita commented Sep 5, 2022

@rossyman you can take as example the mongoose.md where the ApiList is used to display mongoose decorators ;)

@rossyman
Copy link
Contributor Author

rossyman commented Sep 5, 2022

Ah okay so it seems the API is queried from tsed.io -- So makes sense these don't show up yet as they're not released.

@Romakita
Copy link
Collaborator

Romakita commented Sep 6, 2022

Ah okay so it seems the API is queried from tsed.io -- So makes sense these don't show up yet as they're not released.

It's possible to regenerate locally the api.json file by running the command: yarn docs:serve. But this command, works only on linux/mac Os.

I'll checkout your PR to verify if the ApiList is correctly configured for Objection page.

See you
Romain

@rossyman
Copy link
Contributor Author

rossyman commented Sep 6, 2022

Tests are running into an issue with array types. Sadly this issue (microsoft/TypeScript#7169) has been open for 6 years now, so I'll add in some extra parameters to the configuration for relationships that are plural to expose this type.

@rossyman
Copy link
Contributor Author

rossyman commented Sep 6, 2022

I can now confirm that the tests and docs are all building / passing correctly 😄

Screenshot 2022-09-06 at 11 33 17

Screenshot 2022-09-06 at 11 25 13

docs/tutorials/objection.md Outdated Show resolved Hide resolved
@Romakita
Copy link
Collaborator

Romakita commented Sep 7, 2022

@rossyman Great job ;)

Just small cosmetics changes !

See you
Romain

@rossyman
Copy link
Contributor Author

rossyman commented Sep 7, 2022

Good to go! 🚀

Screenshot 2022-09-08 at 00 42 17

@rossyman rossyman force-pushed the feat-objection-relation-decorators branch from f8b7661 to 926a7e7 Compare September 7, 2022 23:49
@rossyman
Copy link
Contributor Author

rossyman commented Sep 7, 2022

Just on a side note, Objection.js isn't listed under the Plugins sidebar section on the docs site. -- Added it in this PR as a separate commit.

@Romakita
Copy link
Collaborator

Romakita commented Sep 8, 2022

@rossyman Perfect PR ;)

Ready to merge for me!

@Romakita Romakita merged commit 902d801 into tsedio:production Sep 8, 2022
@rossyman rossyman deleted the feat-objection-relation-decorators branch September 8, 2022 08:48
@Romakita
Copy link
Collaborator

Romakita commented Sep 8, 2022

🎉 This PR is included in version 6.131.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Romakita
Copy link
Collaborator

🎉 This PR is included in version 7.0.0-rc.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

2 participants