Prisma1: Project download for ejected projects should be put into src folder

[marktani]

Currently, downloading a legacy project uses code folder. Should be renamed to src.

[do4gr]

Currently when ejecting a project we create a folder structure to store files with code/schema/query for functions and permissions.
For this we have ./code for files belonging to functions and ./query for permission queries. This feels a little weird since the code folder then also contains queries and schemata for functions and not all queries are in the query folder as expected.

The yml file will then look inconsistent.
For subscription functions it can for example contain:

• code: ./code/CreateCustomer.js
• query: ./code/CreateCustomer.graphql

And for permission queries:

• query: ./query/CreateCustomer.graphql

We should come up with a consistent and intuitive approach here.

[timsuchanek]

My understanding is that we want to put everything into ./src, the queries and js files.
This approach is the least opinionated and most consistent.
The proposal is to put permission queries into ./permissions to separate these clearly.

[schickling]

• Permission queries should be exported all into one file under src/permissions.graphql and be referenced via GraphQL operation names
• The code and query folder should be unified into a src folder

[marktani]

Permission queries should be exported all into one file under src/permissions.graphql and be referenced via GraphQL operation names

is that possible already, @timsuchanek? how does the syntax for this look like?

[kbrandwijk]

Permission queries should be exported all into one file under src/permissions.graphql and be referenced via GraphQL operation names

Why would you want to combine all of those queries in a single file? I have seen quite a few people create a folder structure per Type. Separate files for each query makes that a lot easier.

[schickling]

Good point, in that case I'd suggest one merged file per type. Here is an example:

.
└── src
    ├── hello.js
    └── permissions
        ├── Post.graphql
        └── User.graphql

[timsuchanek]

How do we want to refer to the operations?
I suggest using : like serverless does:

permissions:
- operation: User.create
  query: ./src/permissions/Post.graphql:create

[kbrandwijk]

Except that it's not a variable. For functions, serverless uses the dot syntax: handler: foo.bar indicates the bar export in foo.js. So query: Post.create would look for query create {...} in Post.graphql.

[marktani]

@timsuchanek I assume in your proposal, the Post.graphql file would look somewhat like this:

query create {
  SomeUserExists
}

query delete {
  SomeUserExists
}

this now conflicts with the topic covered here #703. I suggest to abstain from merging multiple permission queries into one file for now, for exactly this reason.

My suggestion for a short-term change is this:

.
└── src
    ├── hello.js
    └── permissions
        ├── create.graphql
        └── delete.graphql

which would fulfill the original request of streamlining the code vs. src terminology.

Being able to refer to a specific query in one file is unfortunately a bigger task, as I sketched out above and should be tackled at a later point in time.

[do4gr]

I'll try to propose a solution that covers this issue and at the same time allows us to move forward with https://github.com/graphcool/graphcool/issues/456 and https://github.com/graphcool/graphcool/issues/703.

We are aiming for a simple folder structure like this:

.
└── src
    ├── SampleFunction.js
    ├── SampleFunction.graphql
    └── permissions
        ├── Post.graphql
        └── User.graphql

The src folder will contain all files that we generate when downloading a legacy project. These can be the following ones:

• Function queries e.g. SampleFunction.graphql
• Function code e.g. SampleFunction.js
• Function schemata e.g. SampleFunction.graphql

Since we would like to just use the function name as identifier here we need the file ending since functions can have both code and schema / query.

The folder permissions will then contain all permission queries. Within that folder we will combine all permission queries for one type / relation into one file to reduce clutter. For this we need an identifier, I therefore propose that we always expect a valid GraphQL query of the following type for permission queries. The Post.graphql file could then look like this.

query adminUpdatePermission($user_id: ID!) {
  SomeUserExists(
    filter: {
      id: $user_id
      role: ADMIN
    }
  )
}

query adminDeletePermission($user_id: ID!) {
  SomeUserExists(
    filter: {
      id: $user_id
      role: ADMIN
    }
  )
}

This has the advantage that it should play nice with other tooling the user might be using since it is valid in itself

As mentioned in https://github.com/graphcool/graphcool/issues/703 this new format does not work at the moment since the Console provides us with another format and we are only able to handle this old format at the moment. We will adjust the backend to handle both formats so that users don't need to take any action for stored permission queries.

Additionally, we will introduce permission query validation during create and update that check that queries deployed with the CLI adhere to the new format. When downloading legacy projects with the old format we will convert them to the new format in the generated files.

With these proposed changes the graphcool.yml file could then look like this:

types: ./types.graphql
functions: 
  SampleFunction:
    handler:
      code:
        src: ./src/SampleFunction.js
    type: resolver
    schema: ./src/SampleFunction.graphql
permissions:
  - operation: Post.update
     query: ./src/permissions/Post.graphql:adminUpdatePermission
  - operation: Post.delete
     query: ./src/permissions/Post.graphql:adminDeletePermission

[marktani]

lists should be indented:

permissions:
  - operation: Post.update
    query: ./src/permissions/Post.graphql:adminUpdatePermission
  - operation: Post.delete
    query: ./src/permissions/Post.graphql:adminDeletePermission

[lastmjs]

I just want to voice that I think the old console format of permissions without the query header definitely needs to go, since it isn't valid query syntax and may lead to confusion because of that.

[lastmjs]

Also, can we discuss how fields will play into all of this? I sometimes have multiple distinct permission queries that should apply for a certain operation, based on the fields. What I'm doing right now is creating a separate file for each operation based on the fields. It's kind of awkward though, because I end up with file names like the following:

----course
------course-read-all.graphql
------course-read-title-price.graphql

What happens when my permission requirements change and I need to add more specificity based on fields? I will have to go and change potentially multiple file names. It's going to get messy.

I want to bring this up to see if there is a more elegant solution that can be built in.

[kbrandwijk]

:confused:

[marktani]

This is now implemented 🙂 @do4gr will share more details soon.

[do4gr]

We implemented the folder changes in line with https://github.com/graphcool/graphcool/issues/602#issuecomment-333561263

The new permissions format can also already be used. It is not required yet though. Known limitations of the new format are tracked here: https://github.com/graphcool/graphcool/issues/772

[do4gr]

@lastmjs
With the new possibility of addressing queries within one file by name you could bundle your queries in one file per type/relation and would reference them by query name. This way you are working within one file when changing around permissions on a field level and do not need to change filenames.

query readAll { SomeUserExists {...}}

query readTitlePrice { SomeUserExists{...}}

.
└── src
    ├── SampleFunction.js
    ├── SampleFunction.graphql
    └── permissions
        └── Course.graphql

permissions:
  - operation: Course.read
    query: ./src/permissions/Course.graphql:readAll
  - operation: Course.read
    query: ./src/permissions/Course.graphql:readTitlePrice
    fields: [title, price]