maoosi / prisma-appsync

⚡ Turns your ◭ Prisma Schema into a fully-featured GraphQL API, tailored for AWS AppSync.
https://prisma-appsync.vercel.app
BSD 2-Clause "Simplified" License
226 stars 18 forks source link

Question: `<Model>RelationFilter` and `<Model>ScalarWhereInput` vs. `<Model>WhereInput` #83

Closed Tenrys closed 1 year ago

Tenrys commented 1 year ago

Hi,

While using the library and attempting to make deep where filter requests, I noticed that you can't filter on deeper than 1 relation on a model. I looked at the template for the schema generation and fiddled with it on my end like this:

Then, testing locally with a GraphQL request like this:

query MyQuery {
  listEstates(where: {valuation: {mandate: {number: {equals: "1862"}}}}) {
    uuid
    valuation {
      mandate {
        number
      }
    }
  }
}

I was able to get the result I wanted, which is currently not possible with the library's current version. I thought to myself I may as well make a pull request since this is handy, but I figured there could be a reason as to why you deliberately designed things this way, so I'm asking here to see if it's worth considering?

Here are the added commits I'm using currently: https://github.com/maoosi/prisma-appsync/compare/main...Tenrys:prisma-appsync:main The few changes I made probably broke other things but for now it seems to work just like I need it to. I am pretty sure I found a bug with field.isUnique and <Model>WhereUniqueInput too... Prisma 4.8.0 doesn't actually let you use fields generated from relations, so I corrected that.

maoosi commented 1 year ago

Thanks @Tenrys, yes that's worth considering! Assuming that it doesn't break any other existing relation filters.

It is getting quite difficult to test these types of changes manually, so I've started writing tests for the Prisma to GraphQL Schema conversion (see https://github.com/maoosi/prisma-appsync/issues/84). Once this is ready, it will be easier to test your changes on packages/generator/templates/schema.gql.njk and catch potential issues.

In the meantime, please feel free to open a PR - so that I can review and test the suggested changes.

maoosi commented 1 year ago

@Tenrys Quick follow-up question regarding your changes on packages/generator/src/compiler.ts.

I am pretty sure I found a bug with field.isUnique and WhereUniqueInput too... Prisma 4.8.0 doesn't actually let you use fields generated from relations, so I corrected that.

I've just tested this on my end (using 4.8.0) and Prisma DMMF still includes information on relations (but its only visible on fields with actual relations). See example console log below:

model User {
  posts  Post[]
}

model Post {
  author      User?     @relation(fields: [authorUuid], references: [uuid])
  authorUuid  String?   @db.VarChar(200)
}
// DMMF.Field
{
    name: 'posts',
    kind: 'object',
    isList: true,
    isRequired: true,
    isUnique: false,
    isId: false,
    isReadOnly: false,
    hasDefaultValue: false,
    type: 'Post',
    relationName: 'PostToUser',
    relationFromFields: [],
    relationToFields: [],
    isGenerated: false,
    isUpdatedAt: false
},
{
  name: 'author',
  kind: 'object',
  isList: false,
  isRequired: false,
  isUnique: false,
  isId: false,
  isReadOnly: false,
  hasDefaultValue: false,
  type: 'User',
  relationName: 'PostToUser',
  relationFromFields: [ 'authorUuid' ],
  relationToFields: [ 'uuid' ],
  isGenerated: false,
  isUpdatedAt: false
}

Am I missing something?

Tenrys commented 1 year ago

Maybe I explained myself incorrectly, but what I meant to say was that the generated input type for a <Model>WhereUniqueInput doesn't match the TypeScript type that Prisma generates under the same name, which creates an error, so that's why I made this commit: https://github.com/maoosi/prisma-appsync/commit/865c679cb9dcf798b5238cc83e9ba5d9a43b7243

Example:

model Mandate {
  ...
  creator            MandateCreator? @relation(fields: [mandateCreatorUuid], references: [uuid])
  mandateCreatorUuid String?
  ...
}
# Generated GraphQL schema
input MandateWhereUniqueInput {
    number: String
    mandateCreatorUuid: String
    valuationUuid: String
}
// Generated Prisma TypeScript type
export type MandateWhereUniqueInput = {
  number?: string
  valuationUuid?: string
}

Which can cause an error like this if you try to use the extra field:

image image

Also, I feel like I should explain myself regarding this commit: https://github.com/maoosi/prisma-appsync/commit/8ca17689b665aa78a00f82438af18f8555d91cc9

With Prisma schema definitions as such:

model Valuation {
  ...
  mandate     Mandate?
  ...
}

model Mandate {
  ...
  valuation          Valuation       @relation(fields: [valuationUuid], references: [uuid])
  valuationUuid      String          @unique
  ...
}

// Not sure if other fields are actually relevant for this bug

I got this generated ValuationWhereInput:

input ValuationWhereInput {
    ...
    mandate: MandateFilter
    ...
}

# References a one-to-many filter, which doesn't match the specified type of relationship

input MandateFilter {
    some: MandateScalarWhereInput
    every: MandateScalarWhereInput
    none: MandateScalarWhereInput
}

which prompted me to change the function, which gave me the appropriate results on my end.

I made my pull request here: https://github.com/maoosi/prisma-appsync/pull/86

maoosi commented 1 year ago

Hey @Tenrys,

1/ Issue: GraphQL input <Model>WhereUniqueInput shouldn’t include Relation fields

# Generated GraphQL schema
input MandateWhereUniqueInput {
    number: String
    mandateCreatorUuid: String
    valuationUuid: String
}

You are right on this one, mandateCreatorUuid shouldn’t be part of MandateWhereUniqueInput and https://github.com/maoosi/prisma-appsync/commit/865c679cb9dcf798b5238cc83e9ba5d9a43b7243 seems to be the right solve.

2/ Feature: Support for deeply nested relation filters

Right now, you can already do something like:

# relation to one (only one author per comment)
listComments(
  where: {
    author: {
      username: { equals: "username" }
    }
  }
)

Or:

# relation to many (many posts per user)
listUsers(
  where: {
    posts: {
      every: {
        published: { equals: true }
      }
    }
  }
)

However, deeply nested relation filters aren’t supported yet. So the below will not work (yet):

listComments(
  where: {
    author: {
      # deeply nested relation filter
      posts: {
        every: {
          published: { equals: true }
        }
      }
    }
  }
)
listUsers(
  where: {
    posts: {
      every: {
        # deeply nested relation filter
        comments: {
          every: {
            message: { startsWith: 'hello' }
          }
        }
      }
    }
  }
)

From your example below, I believe the generated filter is correct as it is a one-to-many relation (many possible mandates can be associated to the same valuation):

# References a one-to-many filter, which doesn't match the specified type of relationship
input MandateFilter {
    some: MandateScalarWhereInput
    every: MandateScalarWhereInput
    none: MandateScalarWhereInput
}

However, what’s missing is the nested relation filter, because MandateScalarWhereInput does not expose deeply nested relation fields.

From what I understand, packages/generator/templates/schema.gql.njk seems to be an attempt on making it work.

I’ll finalise writing some test for nested relation filter queries and then review your PR accordingly. Thanks for submitting!

Tenrys commented 1 year ago

Quickly replying from my phone:

From your example below, I believe the generated filter is correct as it is a one-to-many relation (many possible mandates can be associated to the same valuation):

But I didn't define it as such in my Prisma schema, Valuation only has a single reference to a single mandate, not a list of mandates, and only one mandate can be assigned to one specific valuation as determined by the @unique directive.

maoosi commented 1 year ago

The generated GraphQL Schema using the following Prisma Schema (this is the one used for Unit Testing) seems correct: https://github.com/maoosi/prisma-appsync/blob/main/tests/prisma/schema.prisma

Note that the below input Filter will be generated for all Models, regardless of whether it is required or in use:

input <Model>Filter {
    some: <Model>ScalarWhereInput
    every: <Model>ScalarWhereInput
    none: <Model>ScalarWhereInput
}

Anyway, I've just merged your PR! So this should now be resolved.