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

ui.hideCreate and ui.hideDelete don't work when graphql.omit.create and graphql.omit.delete are set #9216

Closed
tv42 opened this issue Jul 16, 2024 · 2 comments · Fixed by #9217
Assignees
Labels
🐛 bug Unresolved bug

Comments

@tv42
Copy link

tv42 commented Jul 16, 2024

$ yarn create keystone-app
$ cd keystonejs-bug-hidecreate-hidedelete
$ npm run dev

Open browser. Create first user, navigate to /posts. Observe create button.

Edit schema.ts, add to lists.Post a section ui: { hideCreate: true },.
Observe browser page hide create button.

Add section graphql: {omit: {create: true}},.
Observe browser page start showing create button again.

Same is true for delete, with ui: { hideDelete: true}, and graphql: {omit: {delete: true}},.

Create/delete should stay hidden whenever ui.hideCreate/ui.hideDelete is true. graphql.omit.* should only be a fallback, and not affect the hiding when the ui.* settings are explicitly set.

Current dependencies from yarn create-app:

  "dependencies": {
    "@keystone-6/auth": "^7.0.0",
    "@keystone-6/core": "^5.0.0",
    "@keystone-6/fields-document": "^7.0.0",
    "typescript": "^4.9.5"
  }

Earlier related issue: #8714 (PR #8715).

@tv42
Copy link
Author

tv42 commented Jul 16, 2024

This logic is hilariously wrong. It says do NOT hide the button if graphql API is disabled. Ternary expressions claim another victim.

      hideCreate: normalizeMaybeSessionFunction(
        list.graphql.isEnabled.create ? listConfig.ui?.hideCreate ?? false : false
      ),

Proposed fix:

      hideCreate: normalizeMaybeSessionFunction(
        listConfig.ui?.hideCreate ?? !list.graphql.isEnabled.create,
      ),

That is, use whatever developer said explicitly; if no explicit configuration, hide when GraphQL API is not enabled.

Same for delete.

@dcousens dcousens self-assigned this Jul 16, 2024
@dcousens
Copy link
Member

dcousens commented Jul 17, 2024

@tv42 nice. I'll make a pull request.

I don't blame the ternary expression though, I think our use of inverted boolean logic (hide*, omit) is a pattern that is repeatedly causing downstream [and upstream] errors.

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

Successfully merging a pull request may close this issue.

2 participants