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

decimal field with min or max, resolveInput triggers error #8597

Closed
Thinkscape opened this issue Jun 1, 2023 · 4 comments
Closed

decimal field with min or max, resolveInput triggers error #8597

Thinkscape opened this issue Jun 1, 2023 · 4 comments
Labels
🐛 bug Unresolved bug

Comments

@Thinkscape
Copy link

Thinkscape commented Jun 1, 2023

Steps

  1. Have a list with a decimal field
list({
  fields: {
    lineAmount: decimal({
      validation: { isRequired: false, min: "0.0" },
      defaultValue: "0",
      precision: 10,
      scale: 2,
    }),
});
  1. Have a list resolveInput that changes the value:
list({
  hooks: {
    resolveInput: ({ resolvedData }) {
      return {
        ...resolvedData,
        lineItem: 666
      };
   }
});
  1. Send a create or update against the list

Expected

Items gets created/updated successfully (with the lineItem: 666 value)

Actual

An error occured while running "validateInput".
  - ListName.lineAmount.hooks.validateInput: val.lessThan is not a function

Workarounds

  • Moving the logic to beforeOperation and mutating the resolvedData instance works around the problem.
  • Adding max to field config doesn't help.
  • Removing min max validation from field config works around it (but disables validation)
  • Returning number or string from resolveInput for the decimal field doesn't seem to change anything.
@Thinkscape Thinkscape changed the title decimal field with min, no max, resolveInput triggers error decimal field with min or max, resolveInput triggers error Jun 1, 2023
@dcousens dcousens added the 🐛 bug Unresolved bug label Jun 1, 2023
@borisno2
Copy link
Member

borisno2 commented Jun 6, 2023

This is most likely due to the Decimal validation expecting to be passed a decimal (.lessThan is not available on number or string).

I am assuming if you passed a Decimal into resolveInput validation would succeed for example:

list({
  hooks: {
    resolveInput: ({ resolvedData }) {
      return {
        ...resolvedData,
        lineItem: new Decimal(666)
      };
   }
});

One possible fix in Keystone, to support passing in a number or string into resolve input, would be to parse val in the decimal validation.

@Thinkscape
Copy link
Author

Ah! I shall try that.
If that's the case, then the confusion came from the fact, that beforeOperation is perfectly fine with passing number values, and it casts (wraps) them with Decimal 🤔

@dcousens
Copy link
Member

dcousens commented Sep 3, 2024

Fixed by #9262

@dcousens dcousens closed this as completed Sep 3, 2024
@dcousens
Copy link
Member

dcousens commented Sep 3, 2024

beforeOperation is perfectly fine with passing number values

What do you mean by this? What is passing a number in this situation?
I think our GraphQL resolvers should be only passing a Decimal type through, but maybe Prisma is liberal in what it accepts?

Probably easiest to follow Prisma for this

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

No branches or pull requests

3 participants