From 29529b2c56d4af7c6abce113da2f7ce84f1dcc02 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Date: Thu, 19 Aug 2021 13:54:56 +0200 Subject: [PATCH] fix: improve id type semantic and restrict possible types to text and number --- demo/collections/CustomID.ts | 2 +- docs/configuration/collections.mdx | 2 +- .../views/collections/Edit/formatFields.tsx | 9 ++--- src/collections/buildSchema.ts | 6 ++-- src/collections/config/schema.ts | 5 ++- src/collections/config/types.ts | 6 +++- src/collections/graphql/init.ts | 24 +++++--------- src/collections/operations/create.ts | 2 +- src/graphql/schema/buildMutationInputType.ts | 12 ++----- src/mongoose/buildSchema.ts | 33 +++++++++---------- 10 files changed, 46 insertions(+), 55 deletions(-) diff --git a/demo/collections/CustomID.ts b/demo/collections/CustomID.ts index 94239486f8a..82e570e2716 100644 --- a/demo/collections/CustomID.ts +++ b/demo/collections/CustomID.ts @@ -6,7 +6,7 @@ const CustomID: CollectionConfig = { singular: 'CustomID', plural: 'CustomIDs', }, - id: Number, + idType: 'number', fields: [ { name: 'name', diff --git a/docs/configuration/collections.mdx b/docs/configuration/collections.mdx index a0c66d0a9ba..7f7df0f255f 100644 --- a/docs/configuration/collections.mdx +++ b/docs/configuration/collections.mdx @@ -23,7 +23,7 @@ It's often best practice to write your Collections in separate files and then im | **`access`** | Provide access control functions to define exactly who should be able to do what with Documents in this Collection. [More](/docs/access-control/overview/#collections) | | **`auth`** | Specify options if you would like this Collection to feature authentication. For more, consult the [Authentication](/docs/authentication/config) documentation. | | **`upload`** | Specify options if you would like this Collection to support file uploads. For more, consult the [Uploads](/docs/upload/overview) documentation. | -| **`id`** | Set a [Mongoose-compatible type](https://mongoosejs.com/docs/schematypes.html) to replace the auto-generated id with a customizable value when creating a new record. | +| **`idType`** | Set this option to replace the auto-generated id with a customizable value when creating a new record. Supported values are `text` and `number`. | | **`timestamps`** | Set to false to disable documents' automatically generated `createdAt` and `updatedAt` timestamps. | *\* An asterisk denotes that a property is required.* diff --git a/src/admin/components/views/collections/Edit/formatFields.tsx b/src/admin/components/views/collections/Edit/formatFields.tsx index 7d748041bb4..c2baaba12b5 100644 --- a/src/admin/components/views/collections/Edit/formatFields.tsx +++ b/src/admin/components/views/collections/Edit/formatFields.tsx @@ -1,18 +1,19 @@ import { Field } from '../../../../../fields/config/types'; -import { text } from '../../../../../fields/validations'; +import validations from '../../../../../fields/validations'; const formatFields = (collection, isEditing) => { let fields = [ ...collection.fields, ]; - if (collection.id && !isEditing) { + if (collection.idType && !isEditing) { + const defaultValidate = validations[collection.idType]; fields = [ { name: 'id', - type: 'text', + type: collection.idType, label: 'ID', required: true, - validate: (val) => text(val, { required: true }), + validate: (val) => defaultValidate(val, { required: true }), } as Field, ...fields, ]; diff --git a/src/collections/buildSchema.ts b/src/collections/buildSchema.ts index c08748b2f86..985ec2bd7e3 100644 --- a/src/collections/buildSchema.ts +++ b/src/collections/buildSchema.ts @@ -11,8 +11,10 @@ const buildCollectionSchema = (collection: SanitizedCollectionConfig, config: Sa collection.fields, { timestamps: collection.timestamps !== false, ...schemaOptions }, ); - if (collection.id) { - schema.add({ _id: collection.id }); + + if (collection.idType) { + const idSchemaType = collection.idType === 'number' ? Number : String; + schema.add({ _id: idSchemaType }); } schema.plugin(paginate, { useEstimatedCount: true }) diff --git a/src/collections/config/schema.ts b/src/collections/config/schema.ts index ba4d69758c1..601dfe5f0e8 100644 --- a/src/collections/config/schema.ts +++ b/src/collections/config/schema.ts @@ -7,7 +7,10 @@ const collectionSchema = joi.object().keys({ singular: joi.string(), plural: joi.string(), }), - id: joi.any(), + idType: joi.alternatives().try( + joi.string(), + joi.number(), + ), access: joi.object({ create: joi.func(), read: joi.func(), diff --git a/src/collections/config/types.ts b/src/collections/config/types.ts index 84edb679dd4..00d985b3c0d 100644 --- a/src/collections/config/types.ts +++ b/src/collections/config/types.ts @@ -16,6 +16,10 @@ export interface AuthCollectionModel extends CollectionModel { resetPasswordExpiration: Date; } +export type IdType = + | 'text' + | 'number'; + export type HookOperationType = | 'create' | 'read' @@ -95,7 +99,7 @@ export type CollectionConfig = { singular?: string; plural?: string; }; - id?: any, + idType?: IdType, fields: Field[]; admin?: { useAsTitle?: string; diff --git a/src/collections/graphql/init.ts b/src/collections/graphql/init.ts index 4ebe3efd8b3..7b4ddb982ce 100644 --- a/src/collections/graphql/init.ts +++ b/src/collections/graphql/init.ts @@ -49,18 +49,10 @@ function registerCollections(): void { collection.graphQL = {}; - let idType; - let idFieldType; - switch (collection.config.id) { - case Number: - idType = GraphQLInt; - idFieldType = 'number'; - break; - - default: - idType = GraphQLString; - idFieldType = 'text'; - } + const idType = collection.config.idType === 'number' + ? GraphQLInt + : GraphQLString; + const baseFields: BaseFields = { id: { type: new GraphQLNonNull(idType), @@ -71,10 +63,10 @@ function registerCollections(): void { ...fields, ]; - if (collection.config.id) { + if (collection.config.idType) { whereInputFields.push({ name: 'id', - type: collection.config.id, + type: collection.config.idType, }); } @@ -122,10 +114,10 @@ function registerCollections(): void { }); } - const mutationInputFields = collection.config.id + const mutationInputFields = collection.config.idType ? [{ name: 'id', - type: idFieldType, + type: collection.config.idType, required: true, }, ...fields] : fields; diff --git a/src/collections/operations/create.ts b/src/collections/operations/create.ts index 5552b8d5c31..00c5d003d29 100644 --- a/src/collections/operations/create.ts +++ b/src/collections/operations/create.ts @@ -74,7 +74,7 @@ async function create(this: Payload, incomingArgs: Arguments): Promise // Custom id // ///////////////////////////////////// - if (args.collection.config.id !== undefined) { + if (args.collection.config.idType) { data = { _id: data.id, ...data, diff --git a/src/graphql/schema/buildMutationInputType.ts b/src/graphql/schema/buildMutationInputType.ts index 0e04f4c28b1..8a54089248a 100644 --- a/src/graphql/schema/buildMutationInputType.ts +++ b/src/graphql/schema/buildMutationInputType.ts @@ -19,15 +19,7 @@ import { ArrayField, Field, FieldWithSubFields, GroupField, RelationshipField, R import { toWords } from '../../utilities/formatLabels'; import payload from '../../index'; -const getCollectionIDType = (id) => { - switch (id) { - case Number: - return GraphQLInt; - - default: - return GraphQLString; - } -}; +const getCollectionIDType = (idType) => (idType === 'number' ? GraphQLInt : GraphQLString); function buildMutationInputType(name: string, fields: Field[], parentName: string, forceNullable = false): GraphQLInputObjectType { const fieldToSchemaMap = { @@ -101,7 +93,7 @@ function buildMutationInputType(name: string, fields: Field[], parentName: strin }, }); } else { - type = getCollectionIDType(payload.collections[relationTo].config.id); + type = getCollectionIDType(payload.collections[relationTo].config.idType); } return { type: field.hasMany ? new GraphQLList(type) : type }; diff --git a/src/mongoose/buildSchema.ts b/src/mongoose/buildSchema.ts index 2eae784becd..d37c220b6d6 100644 --- a/src/mongoose/buildSchema.ts +++ b/src/mongoose/buildSchema.ts @@ -343,20 +343,17 @@ const fieldToSchemaMap = { let schemaToReturn: { [key: string]: any } = {}; const relationTo = [].concat(field.relationTo); - const relatedCollections = relationTo.map((slug) => ( - config.collections.find((collection) => collection.slug === slug) - )); - const ids = relatedCollections.map(({ id }) => id); - const areIdTypesConsistent = ids.every((id) => ids[0] === id); - let idType; - if (areIdTypesConsistent) { - if (ids[0] === undefined) { - idType = Schema.Types.ObjectId; - } else { - [idType] = ids; - } - } else { - idType = Schema.Types.Mixed; + const { idType: relatedIdType } = config.collections.find(({ slug }) => slug === relationTo[0]); + let idSchemaType; + switch (relatedIdType) { + case 'text': + idSchemaType = String; + break; + case 'number': + idSchemaType = Number; + break; + default: + idSchemaType = Schema.Types.ObjectId; } if (field.localized) { @@ -367,14 +364,14 @@ const fieldToSchemaMap = { if (hasManyRelations) { localeSchema._id = false; localeSchema.value = { - type: idType, + type: idSchemaType, refPath: `${field.name}.${locale}.relationTo`, }; localeSchema.relationTo = { type: String, enum: field.relationTo }; } else { localeSchema = { ...formatBaseSchema(field), - type: idType, + type: idSchemaType, ref: field.relationTo, }; } @@ -389,7 +386,7 @@ const fieldToSchemaMap = { } else if (hasManyRelations) { schemaToReturn._id = false; schemaToReturn.value = { - type: idType, + type: idSchemaType, refPath: `${field.name}.relationTo`, }; schemaToReturn.relationTo = { type: String, enum: field.relationTo }; @@ -398,7 +395,7 @@ const fieldToSchemaMap = { } else { schemaToReturn = { ...formatBaseSchema(field), - type: idType, + type: idSchemaType, ref: field.relationTo, };