doug-martin / nestjs-query

Easy CRUD for GraphQL.
https://doug-martin.github.io/nestjs-query
MIT License
822 stars 142 forks source link

Nested mutations #1514

Open hghammoud opened 2 years ago

hghammoud commented 2 years ago

I recently stumbled upon this library and trying to find my way inside it. One of the question that i am facing: is there a way to create nested relation from the parent create mutation?

Lets say the object User -> (has) a Profile

@ObjectType({ description: 'user ' })
@Relation('profile', () => Profile, { disableRemove: true })
@Entity()
export class User {
    constructor(partial: Partial<User>) {
        Object.assign(this, partial);
    }

    @IDField(type => ID)
    @PrimaryGeneratedColumn('uuid')
    id: string;

    @FilterableField()
    @Column()
    email: string;

    @Field()
    @Column()
    username: string;

    @OneToOne(() => Profile)
    @JoinColumn()
    profile: Promise<Profile>;

}

is it possible to use the CreateOneUser and embed the profile in the same mutation? if not what other ways can I use to create both the profile, the user and their relation in one call?

hghammoud commented 2 years ago

For those having similar issue this is what worked for me.

  1. I needed to add @Field annotation on the profile field to make it appear the input CreateUser (created automatically by nestjs-query
  2. However the Profile is an Output type so I also need to specify the @InputType on the Profile class else GraphQL was complaining UnhandledPromiseRejectionWarning: Error: Cannot determine a GraphQL input type ("Profile") for the "profile". even tho Profile was added to the nestjs-query module
  3. Use cascade on the profile field to allow the save on User to save the profile
@ObjectType({ description: 'user ' })
@Relation('profile', () => Profile, { disableRemove: true })
@Entity()
export class User {
    constructor(partial: Partial<User>) {
        Object.assign(this, partial);
    }

    @IDField(type => ID)
    @PrimaryGeneratedColumn('uuid')
    id: string;

    @FilterableField()
    @Column()
    email: string;

    @Field()
    @Column()
    username: string;

    @Field()
    @OneToOne(() => Profile, { cascade: true })
    @JoinColumn()
    profile: Promise<Profile>;

}
@ObjectType({ description: 'profile ' })
@InputType("ProfileInput")
@Entity()
export class Profile {
    constructor(partial: Partial<Profile>) {
        Object.assign(this, partial);
    }

    @IDField(type => ID, { nullable: true })
    @PrimaryGeneratedColumn('uuid')
    id: string;

    @FilterableField()
    @Column()
    name: string;

    @Field()
    @Column()
    company: string;

    @Field()
    @Column()
    dob: string;

    @Field()
    @Column()
    address: string;

    @Field()
    @Column()
    about: string;
}

-- Post edited as I found a better way of doing this Another way of addressing this is to create a custom UserInput which used a ProfileInput.

@ObjectType({ description: 'user ' })
@Relation('profile', () => Profile, { disableRemove: true })
@Entity()
export class User {
    constructor(partial: Partial<User>) {
        Object.assign(this, partial);
    }

    @IDField(type => ID)
    @PrimaryGeneratedColumn('uuid')
    id: string;

    @FilterableField()
    @Column()
    email: string;

    @Field()
    @Column()
    username: string;

    @OneToOne(() => Profile, { cascade: true })
    @JoinColumn()
    profile: Promise<Profile>;
}

@InputType()
export class UserInput extends OmitType(User, ['id'], InputType) {
    @Field(type => ProfileInput)
    profile: Promise<Profile>;
}
@ObjectType({ description: 'profile ' })
@Entity()
export class Profile {
    constructor(partial: Partial<Profile>) {
        Object.assign(this, partial);
    }

    @IDField(type => ID)
    @PrimaryGeneratedColumn('uuid')
    id: string;

    @FilterableField()
    @Column()
    name: string;

    @Field()
    @Column()
    company: string;

    @Field()
    @Column()
    dob: string;

    @Field()
    @Column()
    address: string;

    @Field()
    @Column()
    about: string;
}

@InputType()
export class ProfileInput extends OmitType(Profile, ['id'], InputType) { }

and then declare it in the NestjsQueryGraphQLModule as a CreateDTOClass

imports: [
    TypeOrmModule.forFeature([UserRepo, ProfileRepo]),
    NestjsQueryGraphQLModule.forFeature({
      imports: [NestjsQueryTypeOrmModule.forFeature([Profile, User])],
      resolvers: [
        { DTOClass: Profile, EntityClass: Profile },
        { DTOClass: User, EntityClass: User, CreateDTOClass: UserInput },
      ],
    })
  ],
smolinari commented 2 years ago

Let me ask some provocative (but meant to be friendly) questions. Are you really calling up a user's profile data along with their account data? All at the same time? Really? Have you noticed that most systems/ apps don't do this? And do you know the reasons why? 😃

Scott

hghammoud commented 2 years ago

Let me ask some provocative (but meant to be friendly) questions. Are you really calling up a user's profile data along with their account data? All at the same time? Really? Have you noticed that most systems/ apps don't do this? And do you know the reasons why? 😃

Scott

This is only a poc to understand how the library works not a real app. It is just a simple example for illustration purposes.

luca-nardelli commented 2 years ago

:+1: for the feature, I had a similar need in a project I'm working on, maybe this example is clearer.

In my case, I had an Application entity that could have N ordered questions, each of these could have M ordered steps that the user was required to fill. In order to avoid exposing the field used for ordering the questions and steps and to have a simpler API for the client, I wanted to only expose the Application resource at the graphql level, and have the client push full Application objects as a whole (with regular arrays of objects for the questions and steps fields).

In the end, since this was a mobile app, I went with the individual resource approach (i.e. the client sends the Application, each Question and each Step individually) since those correspond to different screens, however if this were a web-app where the user had a big "save" button that saved the whole Application object I would need to make a lot of network requests to create/update all relevant resources (1 Application, N questions, N*M steps), leading to huge overhead.

I also thought about the "cascade" + custom input workaround, however this is specific for Typeorm/SQL and it does not check any of the rules defined in Nestjs-query Authorizers, and this meant that I would have to write custom logic to handle this nested persistence of the resources. I'm not against writing custom logic, however in this case it felt like fighting against he library/framework, since we already have most of our "data access" rules encoded in Authorizers.

At a first glance, the ideal flow when pushing nested resources should be like this:

All of this should happen at the query-service/graphql level, without having to enable any cascade persistence option, so that Authorizer rules can be enforced and so that we can be independent from the persistence layer.

@smolinari what do you think?

smolinari commented 2 years ago

1 Application, N questions, N*M steps

In general, what is the reasoning of the web app working differently than the mobile app? That would be complicating the API and for what reasons? I'd rethink that premise from the start.

But, ok. Let's assume there is a big "save" button, as you say, for the web app. What's the problem of saving the questions and steps data in two different mutations? Or, use a single mutation and service to save them, if atomicity is necessary? What you shouldn't do is try and make the incoming data hierarchical, just so it "fits" into the database.

You say "nested resources", but I'd venture to say, they aren't nested, but rather hierarchical.

As for the storing procedure for the data, that has zero to do with GraphQL. All GraphQL is doing is getting the incoming data to a service. In other words, if you have a special case of needing to save data, then you need to create a custom service. I'd highly doubt you can shoehorn these special needs into nestjs-query's service methods, especially if you need any particular business logic to go with it.

Scott

luca-nardelli commented 2 years ago

Hierarchical is definitely more appropriate as a term. And yeah, GraphQL is not involved, I wrote that but what I really meant was that maybe a general solution could be devised independent of the actual persistence layer.

Regarding my example:

What's the problem of saving the questions and steps data in two different mutations?

Saving questions and steps data in different mutations means making multiple trips to the server because either I need the parent entity id for each child entity (when creating), or because I need to update many resources because the user changed them. So if I have 1 application with 5 questions, each of these with 5 steps and I need to update the whole structure because the user changed a bunch of fields/orderings/etc... I'd need to issue 1 updateOne mutation for the application, 5 updateOne for the questions and 25 updateOne for the steps. This quickly accumulates latency of the calls, whereas if I sent a single mutation the client->server latency part would drop down to basically nothing.

I don't see this as making data hierarchical so that it fits in the database, it is hierarchical. I could definitely use a single mutation with a custom service to save them, but my point is that I think we already have 80% of the building blocks within nestjs-query (persistence logic and authorizers) for the simple scenario of "persisting/updating hierarchical resources", and maybe there could be a way to devise a general-purpose approach. Any special business logic can then go in Assemblers (if it's conversion related) or as hooks after the createX/updateX methods of the query-services.

Another similar situation I encountered is handling user-translatable entities. In some past projects translations were handled with an Entity that had a oneToMany relation with EntityTranslation. When editing translations for the entity, the user had all of them displayed in the app, and the save button saved all translations the user was currently working on. Also in this case, if I had to save N translations I would need to issue N updateOne mutations, one for each of the different translations, whereas I could have done everything in a single server roundtrip. Also in this case, the logic is simply "persisting hierarchical resources", without any special thing going on. Obviously, you could argue that this is poor design and the user should be able to save each translation individually, but that's probably not the point right now 😅