keystone icon indicating copy to clipboard operation
keystone copied to clipboard

Admin UI save button does not disable after saving fails (due to field access permissions) resulting in 400 toast

Open NoTuxNoBux opened this issue 3 years ago • 2 comments

Bug report

Describe the bug

The "Save changes" submit button doesn't disable itself again if you first tried to save values that failed.

To Reproduce

Steps to reproduce the behaviour. See below for a code snippet.

  1. Open the Users admin UI and edit an item that has access control permissions that cause a change of a specific field (group in the example) to fail.
  2. Change the value of the field (group).
  3. Save - you will see an error toast that this is forbidden, which is correct.
  4. Change the group field back to its original value (because you can't change that field, but you may still edit other values).
  5. Watch the save button does not disable again, even though the rest of the field wasn't modified. Clicking it results in a 400 error toast and the save button suddenly "catching up" and disabling.

Expected behaviour

Maybe I'm doing something wrong, but I'd expect the "Save button" to disable again, since no changes were made to the form in the end. It only seems to detect this if you didn't try to save failed values first, or after it hit a 400 error once already.

Screenshots

The two toasts after following the steps to reproduce above:

afbeelding

System information

  • OS: Linux
  • Browser (if applies): Firefox 91
  • Keystone version 2021-08-17

Additional Context

My User configuration is mostly default, with a couple of added fields:

User: list({
    ui: {
        listView: {
            initialColumns: ['name', 'email', 'group', 'anotherRelation', 'someRelation'],
        },
    },
    fields: {
        name: text({ isRequired: true }),
        email: text({ isRequired: true, isUnique: true }),
        password: password({ isRequired: true }),
        group: select({
            access: async (parameters) => {
                return false; // Modifying this field will fail.
            },
            options: [
                { label: 'Group 1', value: 'group1' },
                { label: 'Group 2', value: 'group2' },
                { label: 'Group 3', value: 'group3' },
            ],
            ui: {
                displayMode: 'segmented-control',
            },
        }),
        anotherRelation: relationship({ ref: 'Bar.users', many: false }),
        someRelation: relationship({ ref: 'OtherEntity.user', many: false }),
    },
})

The actual response contains the following:

{
  "errors": [
    {
      "message": "Variable \"$id\" of non-null type \"ID!\" must not be null.",
      "locations": [
        {
          "line": 1,
          "column": 36
        }
      ],
      "extensions": {
        "code": "INTERNAL_SERVER_ERROR",
        "exception": {
          "stacktrace": [
            "GraphQLError: Variable \"$id\" of non-null type \"ID!\" must not be null.",
            "    at _loop (/app/node_modules/graphql/execution/values.js:105:15)",
            "    at coerceVariableValues (/app/node_modules/graphql/execution/values.js:121:16)",
            "    at getVariableValues (/app/node_modules/graphql/execution/values.js:50:19)",
            "    at buildExecutionContext (/app/node_modules/graphql/execution/execute.js:203:61)",
            "    at executeImpl (/app/node_modules/graphql/execution/execute.js:101:20)",
            "    at Object.execute (/app/node_modules/graphql/execution/execute.js:60:35)",
            "    at /app/node_modules/apollo-server-core/src/requestPipeline.ts:571:22",
            "    at Generator.next (<anonymous>)",
            "    at /app/node_modules/apollo-server-core/dist/requestPipeline.js:8:71",
            "    at new Promise (<anonymous>)",
            "    at __awaiter (/app/node_modules/apollo-server-core/dist/requestPipeline.js:4:12)",
            "    at execute (/app/node_modules/apollo-server-core/dist/requestPipeline.js:240:20)",
            "    at Object.<anonymous> (/app/node_modules/apollo-server-core/src/requestPipeline.ts:452:30)",
            "    at Generator.next (<anonymous>)",
            "    at fulfilled (/app/node_modules/apollo-server-core/dist/requestPipeline.js:5:58)",
            "    at runMicrotasks (<anonymous>)"
          ]
        }
      }
    }
  ]
}

NoTuxNoBux avatar Aug 17 '21 09:08 NoTuxNoBux

This also reproduces in the following scenario:

  1. Have a relationship where you can select two existing items, A and B, or leave the field empty.
  2. Set field access control on the field to indicate that selecting item A is not allowed, but empty or B is.
  3. Start editing a record with the field empty.
  4. Select item A and try to save. You will see a toast that this is not allowed.
  5. Now select item B, to correct your data, and get a toast with a 400 error.
  6. Press the save button again, you now get a successful green toast.

In other words: once saving has failed once due to access control, the next save - even if the fields were updated to select allowed values - will always fail. (If, additionally, the newly selected values were the old ones, then the save button does not disable, as described above.)

NoTuxNoBux avatar Aug 18 '21 16:08 NoTuxNoBux

Thanks for this detailed report @NoTuxNoBux, and apologies for the delay in responding, we'll look into this shortly.

bladey avatar Sep 10 '21 04:09 bladey