jordanoverbye commented 4 years ago

Bug report

Describe the bug

If you call the addValidationError function that is passed into the validateInput hook, I would expect that the notification message will get shown to the User in the Admin UI App. It currently always says:

GraphQL error
You attempted to perform an invalid mutation

Also, even though the item didn't update, a the success notification ALSO appears saying:

{_label_}
Saved successfully

This shouldn't appear because the Item didn't actually update.

To Reproduce

This issue is easy to reproduce using the todo example as a base.

const { Keystone } = require('@keystonejs/keystone');
const { MongooseAdapter } = require('@keystonejs/adapter-mongoose');
const { Text } = require('@keystonejs/fields');
const { GraphQLApp } = require('@keystonejs/app-graphql');
const { AdminUIApp } = require('@keystonejs/app-admin-ui');
const { StaticApp } = require('@keystonejs/app-static');

const keystone = new Keystone({
  name: 'Keystone To-Do List',
  adapter: new MongooseAdapter(),
});

keystone.createList('Todo', {
  fields: {
    name: { type: Text, isRequired: true },
  },
  hooks: {
    validateInput: async ({ operation, resolvedData, addValidationError }) => {
      if (operation === 'update' && name === 'test') {
      // If the user is trying to update an item and the name is `test`, throw an error
        addValidationError('Hello there!');
      }
    },
  },
});

module.exports = {
  keystone,
  apps: [
    new GraphQLApp(),
    new StaticApp({ path: '/', src: 'public' }),
    // Setup the optional Admin UI
    new AdminUIApp({ enableDefaultRoute: true }),
  ],
};

Expected behaviour

The success toast doesn't show
The message passed into addValidationError gets shown to the user

Screenshots

System information

OS: [e.g. macOS, Windows]
Browser (if applies) [e.g. chrome, safari]

Additional context

Add any other context about the problem here.

gautamsi commented 4 years ago

I have traced this to following lines

https://github.com/keystonejs/keystone/blob/master/packages/app-admin-ui/client/pages/Item/index.js#L221-L232

the updateItem mutation is coming from props and does not throw error (react-apollo thing), it is always success even if the error is thrown. you will see both error and success.