From e3fefafcce6f8bf836c9bf0f4d931b8200ba41c7 Mon Sep 17 00:00:00 2001 From: Charles Date: Tue, 17 Aug 2021 10:27:45 +1000 Subject: [PATCH] 6261/fix delete alert (#6296) * refactor confirm procedure to only add success toast on success * new deletion logic in Listview * add crud-notifications test project * update deletion solution to be more pragmatic at scale * update bug fix to be more verbose * update schema.graphql * minor updates * fix yarn lint:examples to not break when running more than one test-project * minor updates to copy * remove log * changeset Co-authored-by: Tim Leslie --- .changeset/stale-spiders-try.md | 5 + package.json | 2 +- .../admin-ui/pages/ItemPage/index.tsx | 14 +- .../admin-ui/pages/ListPage/index.tsx | 91 ++++- .../crud-notifications/README.md | 4 + .../crud-notifications/keystone.ts | 23 ++ .../crud-notifications/package.json | 22 ++ .../crud-notifications/schema.graphql | 329 ++++++++++++++++++ .../crud-notifications/schema.prisma | 27 ++ .../crud-notifications/schema.ts | 38 ++ 10 files changed, 532 insertions(+), 23 deletions(-) create mode 100644 .changeset/stale-spiders-try.md create mode 100644 tests/test-projects/crud-notifications/README.md create mode 100644 tests/test-projects/crud-notifications/keystone.ts create mode 100644 tests/test-projects/crud-notifications/package.json create mode 100644 tests/test-projects/crud-notifications/schema.graphql create mode 100644 tests/test-projects/crud-notifications/schema.prisma create mode 100644 tests/test-projects/crud-notifications/schema.ts diff --git a/.changeset/stale-spiders-try.md b/.changeset/stale-spiders-try.md new file mode 100644 index 00000000000..24ff010029b --- /dev/null +++ b/.changeset/stale-spiders-try.md @@ -0,0 +1,5 @@ +--- +'@keystone-next/keystone': patch +--- + +Fixed delete success notifications in the Admin UI appearing on failed deletes in List view and Item view. diff --git a/package.json b/package.json index eddabeeb0c7..0cbf8275d20 100644 --- a/package.json +++ b/package.json @@ -35,7 +35,7 @@ "update": "manypkg upgrade", "no-cypress-install": "cross-env CYPRESS_INSTALL_BINARY=0 yarn", "postinstall-examples": "for d in `find examples -type d -maxdepth 1 -mindepth 1`; do cd $d; yarn keystone-next postinstall --fix; cd ../..; done; for d in `find examples-staging -type d -maxdepth 1 -mindepth 1`; do cd $d; yarn keystone-next postinstall --fix; cd ../..; done; for d in `find tests/test-projects -type d -maxdepth 1 -mindepth 1`; do cd $d; yarn keystone-next postinstall --fix; cd ../..; done", - "lint:examples": "for d in `find examples -type d -maxdepth 1 -mindepth 1`; do cd $d; echo $d; SKIP_PROMPTS=1 yarn keystone-next postinstall; if [ $? -ne 0 ]; then exit 1; fi; cd ../..; done; for d in `find examples-staging -type d -maxdepth 1 -mindepth 1`; do cd $d; echo $d; SKIP_PROMPTS=1 yarn keystone-next postinstall; if [ $? -ne 0 ]; then exit 1; fi; cd ../..; done; for d in `find tests/test-projects -type d -maxdepth 1 -mindepth 1`; do cd $d; echo $d; SKIP_PROMPTS=1 yarn keystone-next postinstall; if [ $? -ne 0 ]; then exit 1; fi; cd ../..; done", + "lint:examples": "for d in `find examples -type d -maxdepth 1 -mindepth 1`; do cd $d; echo $d; SKIP_PROMPTS=1 yarn keystone-next postinstall; if [ $? -ne 0 ]; then exit 1; fi; cd ../..; done; for d in `find examples-staging -type d -maxdepth 1 -mindepth 1`; do cd $d; echo $d; SKIP_PROMPTS=1 yarn keystone-next postinstall; if [ $? -ne 0 ]; then exit 1; fi; cd ../..; done; for d in `find tests/test-projects -type d -maxdepth 1 -mindepth 1`; do cd $d; echo $d; SKIP_PROMPTS=1 yarn keystone-next postinstall; if [ $? -ne 0 ]; then exit 1; fi; cd ../../..; done", "generate-filters": "cd prisma-utils && yarn generate", "lint:filters": "cd prisma-utils && yarn verify" }, diff --git a/packages/keystone/src/___internal-do-not-use-will-break-in-patch/admin-ui/pages/ItemPage/index.tsx b/packages/keystone/src/___internal-do-not-use-will-break-in-patch/admin-ui/pages/ItemPage/index.tsx index 5ea383f38be..a49288a34c7 100644 --- a/packages/keystone/src/___internal-do-not-use-will-break-in-patch/admin-ui/pages/ItemPage/index.tsx +++ b/packages/keystone/src/___internal-do-not-use-will-break-in-patch/admin-ui/pages/ItemPage/index.tsx @@ -253,17 +253,19 @@ function DeleteButton({ confirm: { label: 'Delete', action: async () => { - await deleteItem().catch(err => { - toasts.addToast({ - title: 'Failed to delete item', + try { + await deleteItem(); + } catch (err) { + return toasts.addToast({ + title: `Failed to delete ${list.singular} item: ${itemLabel}`, message: err.message, tone: 'negative', }); - }); + } router.push(`/${list.path}`); - toasts.addToast({ + return toasts.addToast({ title: itemLabel, - message: 'Deleted successfully', + message: `Deleted ${list.singular} item successfully`, tone: 'positive', }); }, diff --git a/packages/keystone/src/___internal-do-not-use-will-break-in-patch/admin-ui/pages/ListPage/index.tsx b/packages/keystone/src/___internal-do-not-use-will-break-in-patch/admin-ui/pages/ListPage/index.tsx index f726c32db26..a3f1e6203ca 100644 --- a/packages/keystone/src/___internal-do-not-use-will-break-in-patch/admin-ui/pages/ListPage/index.tsx +++ b/packages/keystone/src/___internal-do-not-use-will-break-in-patch/admin-ui/pages/ListPage/index.tsx @@ -435,14 +435,18 @@ function DeleteManyButton({ useMemo( () => gql` - mutation($where: [${list.gqlNames.whereUniqueInputName}!]!) { - ${list.gqlNames.deleteManyMutationName}(where: $where) { - id - } - } + mutation($where: [${list.gqlNames.whereUniqueInputName}!]!) { + ${list.gqlNames.deleteManyMutationName}(where: $where) { + id + ${list.labelField} + } + } `, [list] - ) + ), + { + errorPolicy: 'all', + } ); const [isOpen, setIsOpen] = useState(false); const toasts = useToasts(); @@ -466,20 +470,75 @@ function DeleteManyButton({ confirm: { label: 'Delete', action: async () => { - await deleteItems({ + const { data, errors } = await deleteItems({ variables: { where: [...selectedItems].map(id => ({ id })) }, - }).catch(err => { + }); + /* + Data returns an array where successful deletions are item objects + and unsuccessful deletions are null values. + Run a reduce to count success and failure as well as + to generate the success message to be passed to the success toast + */ + const { successfulItems, unsuccessfulItems, successMessage } = data[ + list.gqlNames.deleteManyMutationName + ].reduce( + ( + acc: { + successfulItems: number; + unsuccessfulItems: number; + successMessage: string; + }, + curr: any + ) => { + if (curr) { + acc.successfulItems++; + acc.successMessage = + acc.successMessage === '' + ? (acc.successMessage += curr.label) + : (acc.successMessage += `, ${curr.label}`); + } else { + acc.unsuccessfulItems++; + } + return acc; + }, + { successfulItems: 0, unsuccessfulItems: 0, successMessage: '' } as { + successfulItems: number; + unsuccessfulItems: number; + successMessage: string; + } + ); + + // If there are errors + if (errors?.length) { + // Find out how many items failed to delete. + // Reduce error messages down to unique instances, and append to the toast as a message. toasts.addToast({ - title: 'Failed to delete items', - message: err.message, tone: 'negative', + title: `Failed to delete ${unsuccessfulItems} of ${ + data[list.gqlNames.deleteManyMutationName].length + } ${list.plural}`, + message: errors + .reduce((acc, error) => { + if (acc.indexOf(error.message) < 0) { + acc.push(error.message); + } + return acc; + }, [] as string[]) + .join('\n'), }); - }); - toasts.addToast({ - title: 'Deleted items successfully', - tone: 'positive', - }); - refetch(); + } + + if (successfulItems) { + toasts.addToast({ + tone: 'positive', + title: `Deleted ${successfulItems} of ${ + data[list.gqlNames.deleteManyMutationName].length + } ${list.plural} successfully`, + message: successMessage, + }); + } + + return refetch(); }, }, cancel: { diff --git a/tests/test-projects/crud-notifications/README.md b/tests/test-projects/crud-notifications/README.md new file mode 100644 index 00000000000..60e641288cb --- /dev/null +++ b/tests/test-projects/crud-notifications/README.md @@ -0,0 +1,4 @@ +## THIS IS A TEST PROJECT + +The sole purpose of this project is to act as a fixture through which we run our admin-ui integration tests. +For useful and applicable examples of how to use keystone, please visit the [examples directory](https://github.com/keystonejs/keystone/tree/master/examples/) or visit our [docs](https://next.keystonejs.com). diff --git a/tests/test-projects/crud-notifications/keystone.ts b/tests/test-projects/crud-notifications/keystone.ts new file mode 100644 index 00000000000..87ce93e9453 --- /dev/null +++ b/tests/test-projects/crud-notifications/keystone.ts @@ -0,0 +1,23 @@ +import { config } from '@keystone-next/keystone/schema'; +import { lists } from './schema'; + +export default config({ + db: { + provider: 'sqlite', + url: process.env.DATABASE_URL || 'file:./test.db', + async onConnect(context) { + await context.lists.Task.createMany({ + data: [...Array.from(Array(50).keys())].map(key => { + return { label: `do not delete ${key}` }; + }), + }); + + await context.lists.Task.createMany({ + data: [...Array.from(Array(25).keys())].map(key => { + return { label: `deletable ${key}` }; + }), + }); + }, + }, + lists, +}); diff --git a/tests/test-projects/crud-notifications/package.json b/tests/test-projects/crud-notifications/package.json new file mode 100644 index 00000000000..ea5f2c1725c --- /dev/null +++ b/tests/test-projects/crud-notifications/package.json @@ -0,0 +1,22 @@ +{ + "name": "@keystone-next/test-projects-crud-notifications", + "version": "0.0.2", + "private": true, + "license": "MIT", + "scripts": { + "dev": "keystone-next dev", + "start": "keystone-next start", + "build": "keystone-next build" + }, + "dependencies": { + "@keystone-next/fields": "^13.0.0", + "@keystone-next/keystone": "^23.0.0" + }, + "devDependencies": { + "typescript": "^4.3.5" + }, + "engines": { + "node": "^12.20 || >= 14.13" + }, + "repository": "https://github.com/keystonejs/keystone/tree/master/tests/test-projects/crud-notifications" +} diff --git a/tests/test-projects/crud-notifications/schema.graphql b/tests/test-projects/crud-notifications/schema.graphql new file mode 100644 index 00000000000..e194b7cb406 --- /dev/null +++ b/tests/test-projects/crud-notifications/schema.graphql @@ -0,0 +1,329 @@ +type Task { + id: ID! + label: String + priority: TaskPriorityType + isComplete: Boolean + assignedTo: Person + finishBy: String +} + +enum TaskPriorityType { + low + medium + high +} + +input TaskWhereInput { + AND: [TaskWhereInput!] + OR: [TaskWhereInput!] + NOT: [TaskWhereInput!] + id: IDFilter + label: StringNullableFilter + priority: TaskPriorityTypeNullableFilter + isComplete: BooleanNullableFilter + assignedTo: PersonWhereInput + finishBy: DateTimeNullableFilter +} + +input IDFilter { + equals: ID + in: [ID!] + notIn: [ID!] + lt: ID + lte: ID + gt: ID + gte: ID + not: IDFilter +} + +input StringNullableFilter { + equals: String + in: [String!] + notIn: [String!] + lt: String + lte: String + gt: String + gte: String + contains: String + startsWith: String + endsWith: String + not: NestedStringNullableFilter +} + +input NestedStringNullableFilter { + equals: String + in: [String!] + notIn: [String!] + lt: String + lte: String + gt: String + gte: String + contains: String + startsWith: String + endsWith: String + not: NestedStringNullableFilter +} + +input TaskPriorityTypeNullableFilter { + equals: TaskPriorityType + in: [TaskPriorityType!] + notIn: [TaskPriorityType!] + not: TaskPriorityTypeNullableFilter +} + +input BooleanNullableFilter { + equals: Boolean + not: BooleanNullableFilter +} + +input DateTimeNullableFilter { + equals: String + in: [String!] + notIn: [String!] + lt: String + lte: String + gt: String + gte: String + not: DateTimeNullableFilter +} + +input TaskWhereUniqueInput { + id: ID +} + +input TaskOrderByInput { + id: OrderDirection + label: OrderDirection + priority: OrderDirection + isComplete: OrderDirection + finishBy: OrderDirection +} + +enum OrderDirection { + asc + desc +} + +input TaskUpdateInput { + label: String + priority: TaskPriorityType + isComplete: Boolean + assignedTo: PersonRelateToOneForUpdateInput + finishBy: String +} + +input PersonRelateToOneForUpdateInput { + create: PersonCreateInput + connect: PersonWhereUniqueInput + disconnect: Boolean +} + +input TaskUpdateArgs { + where: TaskWhereUniqueInput! + data: TaskUpdateInput! +} + +input TaskCreateInput { + label: String + priority: TaskPriorityType + isComplete: Boolean + assignedTo: PersonRelateToOneForCreateInput + finishBy: String +} + +input PersonRelateToOneForCreateInput { + create: PersonCreateInput + connect: PersonWhereUniqueInput +} + +type Person { + id: ID! + name: String + tasks( + where: TaskWhereInput! = {} + orderBy: [TaskOrderByInput!]! = [] + take: Int + skip: Int! = 0 + ): [Task!] + tasksCount(where: TaskWhereInput! = {}): Int +} + +input PersonWhereInput { + AND: [PersonWhereInput!] + OR: [PersonWhereInput!] + NOT: [PersonWhereInput!] + id: IDFilter + name: StringNullableFilter + tasks: TaskManyRelationFilter +} + +input TaskManyRelationFilter { + every: TaskWhereInput + some: TaskWhereInput + none: TaskWhereInput +} + +input PersonWhereUniqueInput { + id: ID +} + +input PersonOrderByInput { + id: OrderDirection + name: OrderDirection +} + +input PersonUpdateInput { + name: String + tasks: TaskRelateToManyForUpdateInput +} + +input TaskRelateToManyForUpdateInput { + disconnect: [TaskWhereUniqueInput!] + set: [TaskWhereUniqueInput!] + create: [TaskCreateInput!] + connect: [TaskWhereUniqueInput!] +} + +input PersonUpdateArgs { + where: PersonWhereUniqueInput! + data: PersonUpdateInput! +} + +input PersonCreateInput { + name: String + tasks: TaskRelateToManyForCreateInput +} + +input TaskRelateToManyForCreateInput { + create: [TaskCreateInput!] + connect: [TaskWhereUniqueInput!] +} + +""" +The `JSON` scalar type represents JSON values as specified by [ECMA-404](http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf). +""" +scalar JSON + @specifiedBy( + url: "http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-404.pdf" + ) + +type Mutation { + createTask(data: TaskCreateInput!): Task + createTasks(data: [TaskCreateInput!]!): [Task] + updateTask(where: TaskWhereUniqueInput!, data: TaskUpdateInput!): Task + updateTasks(data: [TaskUpdateArgs!]!): [Task] + deleteTask(where: TaskWhereUniqueInput!): Task + deleteTasks(where: [TaskWhereUniqueInput!]!): [Task] + createPerson(data: PersonCreateInput!): Person + createPeople(data: [PersonCreateInput!]!): [Person] + updatePerson(where: PersonWhereUniqueInput!, data: PersonUpdateInput!): Person + updatePeople(data: [PersonUpdateArgs!]!): [Person] + deletePerson(where: PersonWhereUniqueInput!): Person + deletePeople(where: [PersonWhereUniqueInput!]!): [Person] +} + +type Query { + tasks( + where: TaskWhereInput! = {} + orderBy: [TaskOrderByInput!]! = [] + take: Int + skip: Int! = 0 + ): [Task!] + task(where: TaskWhereUniqueInput!): Task + tasksCount(where: TaskWhereInput! = {}): Int + people( + where: PersonWhereInput! = {} + orderBy: [PersonOrderByInput!]! = [] + take: Int + skip: Int! = 0 + ): [Person!] + person(where: PersonWhereUniqueInput!): Person + peopleCount(where: PersonWhereInput! = {}): Int + keystone: KeystoneMeta! +} + +type KeystoneMeta { + adminMeta: KeystoneAdminMeta! +} + +type KeystoneAdminMeta { + enableSignout: Boolean! + enableSessionItem: Boolean! + lists: [KeystoneAdminUIListMeta!]! + list(key: String!): KeystoneAdminUIListMeta +} + +type KeystoneAdminUIListMeta { + key: String! + itemQueryName: String! + listQueryName: String! + hideCreate: Boolean! + hideDelete: Boolean! + path: String! + label: String! + singular: String! + plural: String! + description: String + initialColumns: [String!]! + pageSize: Int! + labelField: String! + fields: [KeystoneAdminUIFieldMeta!]! + initialSort: KeystoneAdminUISort + isHidden: Boolean! +} + +type KeystoneAdminUIFieldMeta { + path: String! + label: String! + isOrderable: Boolean! + fieldMeta: JSON + viewsIndex: Int! + customViewsIndex: Int + createView: KeystoneAdminUIFieldMetaCreateView! + listView: KeystoneAdminUIFieldMetaListView! + itemView(id: ID!): KeystoneAdminUIFieldMetaItemView + search: QueryMode +} + +type KeystoneAdminUIFieldMetaCreateView { + fieldMode: KeystoneAdminUIFieldMetaCreateViewFieldMode! +} + +enum KeystoneAdminUIFieldMetaCreateViewFieldMode { + edit + hidden +} + +type KeystoneAdminUIFieldMetaListView { + fieldMode: KeystoneAdminUIFieldMetaListViewFieldMode! +} + +enum KeystoneAdminUIFieldMetaListViewFieldMode { + read + hidden +} + +type KeystoneAdminUIFieldMetaItemView { + fieldMode: KeystoneAdminUIFieldMetaItemViewFieldMode! +} + +enum KeystoneAdminUIFieldMetaItemViewFieldMode { + edit + read + hidden +} + +enum QueryMode { + default + insensitive +} + +type KeystoneAdminUISort { + field: String! + direction: KeystoneAdminUISortDirection! +} + +enum KeystoneAdminUISortDirection { + ASC + DESC +} diff --git a/tests/test-projects/crud-notifications/schema.prisma b/tests/test-projects/crud-notifications/schema.prisma new file mode 100644 index 00000000000..a1efaced2f7 --- /dev/null +++ b/tests/test-projects/crud-notifications/schema.prisma @@ -0,0 +1,27 @@ +datasource sqlite { + url = env("DATABASE_URL") + provider = "sqlite" +} + +generator client { + provider = "prisma-client-js" + output = "node_modules/.prisma/client" +} + +model Task { + id String @id @default(cuid()) + label String? + priority String? + isComplete Boolean? + assignedTo Person? @relation("Task_assignedTo", fields: [assignedToId], references: [id]) + assignedToId String? @map("assignedTo") + finishBy DateTime? + + @@index([assignedToId]) +} + +model Person { + id String @id @default(cuid()) + name String? + tasks Task[] @relation("Task_assignedTo") +} \ No newline at end of file diff --git a/tests/test-projects/crud-notifications/schema.ts b/tests/test-projects/crud-notifications/schema.ts new file mode 100644 index 00000000000..8cc4f22288c --- /dev/null +++ b/tests/test-projects/crud-notifications/schema.ts @@ -0,0 +1,38 @@ +import { createSchema, list } from '@keystone-next/keystone/schema'; +import { checkbox, relationship, text, timestamp } from '@keystone-next/fields'; +import { select } from '@keystone-next/fields'; + +export const lists = createSchema({ + Task: list({ + access: { + delete: async ({ itemId, context }) => { + const item: any = await context.lists.Task.findOne({ + where: { id: itemId }, + query: 'label', + }); + const matchString = item.label.replace(/([\d])+/g, '').trim(); + return !['do not delete', 'do not destroy', 'do not kill'].includes(matchString); + }, + }, + fields: { + label: text({ isRequired: true }), + priority: select({ + dataType: 'enum', + options: [ + { label: 'Low', value: 'low' }, + { label: 'Medium', value: 'medium' }, + { label: 'High', value: 'high' }, + ], + }), + isComplete: checkbox(), + assignedTo: relationship({ ref: 'Person.tasks', many: false }), + finishBy: timestamp(), + }, + }), + Person: list({ + fields: { + name: text({ isRequired: true }), + tasks: relationship({ ref: 'Task.assignedTo', many: true }), + }, + }), +});