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

When creating items, if validation fails any relationship items remain. #7217

Closed
qwacko opened this issue Jan 23, 2022 · 5 comments
Closed

Comments

@qwacko
Copy link

qwacko commented Jan 23, 2022

Created three schema items (Parent, Child, Grandchild) - as below, And inserted them into the overall keystone schema.

import { list } from "@keystone-6/core";
import { text, relationship } from "@keystone-6/core/fields";
import { ListHooks, BaseListTypeInfo } from "@keystone-6/core/types";

const hooks = (title: string): ListHooks<BaseListTypeInfo> | undefined => ({
  validateInput: async () => {
    console.log(`${title} : validateInput`);
  },
  resolveInput: async ({ resolvedData }) => {
    console.log(`${title} : resolveInput`);
    return resolvedData;
  },
  validateDelete: async () => {
    console.log(`${title} : validateDelete`);
  },
  beforeOperation: async () => {
    console.log(`${title} : beforeOperation`);
  },
  afterOperation: async () => {
    console.log(`${title} : afterOperation`);
  },
});

export const Parent = list({
  fields: {
    title: text({ validation: { isRequired: true } }),
    children: relationship({
      ref: "Child.parent",
      many: true,
    }),
  },
  hooks: hooks("Parent"),
});

export const Child = list({
  fields: {
    title: text(),
    parent: relationship({
      ref: "Parent.children",
      many: false,
    }),
    children: relationship({
      ref: "Grandchild.parent",
      many: true,
    }),
  },
  hooks: hooks("Child"),
});

export const Grandchild = list({
  fields: {
    title: text(),
    parent: relationship({
      ref: "Child.children",
      many: false,
    }),
  },
  hooks: hooks("Grandchild"),
});

Run the following graphql query

mutation {
  createParent(
    data: {
      children: {
        create: [
          { title: "Child", children: { create: [{ title: "Grandchild" }] } }
        ]
      }
    }
  ) {
    id
    title
    children {
      id
      title
      children {
        id
        title
      }
    }
  }
}

This will result in a failure (due to there being no "title" of the Parent object), however the "Child" and "Grandchild" item will still be created.

The count can be checked using the following command (which indicates the Child and Grandchild counts increasing but not the parent count.

query {
  grandchildrenCount
  childrenCount
  parentsCount
}

Describe what you would expect to happen

I would expect that on failure of the validation of any part of a single mutation query (including multiple mutations within a single query), there would be no changes made to the DB.

It may be possibly to undo the changes through manual logic, but due to the ability to create / modify nested items easily, I would imagine performing this change in the provided hooks would become overly complex.

Other useful information

Keystone version = "@keystone-6/core": "^1.0.1"

With the logging that I implemented in the hooks, it would seem that the validation is run on the bottom most item, and works its way back. The code output in the console fo rthe above query is as follows:

Grandchild : resolveInput
Grandchild : validateInput
Grandchild : beforeOperation
Child : resolveInput
Child : validateInput
Child : beforeOperation
Parent : resolveInput
Parent : validateInput

Notice that the first hook called is the grandchild resolving input, and so there is no way thoguh the existing hooks to intervene and check the entire query before enacting any changes.

@dcousens
Copy link
Member

dcousens commented Feb 10, 2022

The question distilled

To what extent do we have database transactions within singular mutation queries, and if not, why not?

@molomby
Copy link
Member

molomby commented Feb 17, 2022

We currently don't use transactions in single mutations; it's a known problem we're looking to rectify.

@molomby
Copy link
Member

molomby commented Feb 17, 2022

As to the "why" – due to the way nested mutations and hooks interact it's not possible to support transactions when using auto increments. When wrestling with the tradeoffs during dev, it was decided to remove transactions in the short term for simplicity sake.

We intend to re-enable transactions by default in cases when the lists being modified don't include any auto incs. For cases where the lists do includes an auto inc (ie. as the list id, with db.idField.kind = 'autoincrement', or as an integer field with defaultValue.kind = 'autoincrement') transactions will still not be supported.

@qwacko
Copy link
Author

qwacko commented Feb 18, 2022

Thanks for the clear explanation of this and great to hear that it is being worked on. I can see how validating a complete transaction in KeystoneJS, and then compiling a Prisma transaction to execute would be pretty difficult.

My hacky workaround which worked for me was to implement this as a custom mutation, which took a bit of work but still ended up benefiting from the types / prisma client / authentication from Keystone JS. In implementing this I possibly had some conditions that can't be validated in Prisma / DB configuration, but needed to be validated in logic. Items like, make sure at least one user is marked as an admin of this group, or make sure there are at least two relationship items that match a specific criteria including when the item is created (where there can also be ones that don't match a criteria). Not sure how this could be implemented in the provided hook structure, as you need to be able to look at what is in the database, as well as modifications possibly being made to relationship items

Possibly I was pushing this system too much, and should have just started with custom mutations, because it is setup more as a CMS, but i was trying to use it as a full app backend.

@dcousens
Copy link
Member

dcousens commented Nov 9, 2022

Unfortunate as the answer is for now, I am closing this question as a duplicate of the following #7642, I have opened a feature request here (#8084).

@dcousens dcousens closed this as completed Nov 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants