Fix creating new collections

api/src/routes/collections.ts

@@ -1,38 +1,23 @@
 import { Router } from 'express';
 import asyncHandler from 'express-async-handler';
 import sanitizeQuery from '../middleware/sanitize-query';
-import * as CollectionsService from '../services/collections';
+import CollectionsService from '../services/collections';
 import { schemaInspector } from '../database';
-import { InvalidPayloadException, CollectionNotFoundException } from '../exceptions';
-import Joi from 'joi';
+import { CollectionNotFoundException } from '../exceptions';
 import useCollection from '../middleware/use-collection';
 
 const router = Router();
 
-const fieldSchema = Joi.object({
-	field: Joi.string().required(),
-	datatype: Joi.string().required(),
-	note: Joi.string().required(),
-	primary_key: Joi.boolean(),
-	auto_increment: Joi.boolean(),
-});
-
-const collectionSchema = Joi.object({
-	collection: Joi.string().required(),
-	fields: Joi.array().items(fieldSchema).min(1).unique().required(),
-	note: Joi.string(),
-});
-
 router.post(
 	'/',
 	useCollection('directus_collections'),
 	asyncHandler(async (req, res) => {
-		const { error } = collectionSchema.validate(req.body);
-		if (error) throw new InvalidPayloadException(error.message);
+		const collectionsService = new CollectionsService({ accountability: req.accountability });
 
-		const createdCollection = await CollectionsService.create(req.body, req.accountability);
+		const collectionKey = await collectionsService.create(req.body);
+		const record = await collectionsService.readByKey(collectionKey);
 
-		res.json({ data: createdCollection || null });
+		res.json({ data: record || null });
 	})
 );
 
@@ -41,10 +26,9 @@ router.get(
 	useCollection('directus_collections'),
 	sanitizeQuery,
 	asyncHandler(async (req, res) => {
-		const collections = await CollectionsService.readAll(
-			req.sanitizedQuery,
-			req.accountability
-		);
+		const collectionsService = new CollectionsService({ accountability: req.accountability });
+		const collections = await collectionsService.readByQuery(req.sanitizedQuery);
+
 		res.json({ data: collections || null });
 	})
 );
@@ -54,15 +38,17 @@ router.get(
 	useCollection('directus_collections'),
 	sanitizeQuery,
 	asyncHandler(async (req, res) => {
+		/** @todo move this validation to CollectionsService methods */
 		const exists = await schemaInspector.hasTable(req.params.collection);
-
 		if (exists === false) throw new CollectionNotFoundException(req.params.collection);
 
-		const collection = await CollectionsService.readOne(
+		const collectionsService = new CollectionsService({ accountability: req.accountability });
+
+		const collection = await collectionsService.readByKey(
 			req.params.collection,
-			req.sanitizedQuery,
-			req.accountability
+			req.sanitizedQuery
 		);
+
 		res.json({ data: collection || null });
 	})
 );
@@ -71,11 +57,13 @@ router.delete(
 	'/:collection',
 	useCollection('directus_collections'),
 	asyncHandler(async (req, res) => {
+		/** @todo move this validation to CollectionsService methods */
 		if ((await schemaInspector.hasTable(req.params.collection)) === false) {
 			throw new CollectionNotFoundException(req.params.collection);
 		}
 
-		await CollectionsService.deleteCollection(req.params.collection, req.accountability);
+		const collectionsService = new CollectionsService({ accountability: req.accountability });
+		await collectionsService.delete(req.params.collection);
 
 		res.end();
 	})

api/src/services/collections.ts

@@ -1,135 +1,77 @@
 import database, { schemaInspector } from '../database';
-import ItemsService from '../services/items';
 import { Query } from '../types/query';
-import { ColumnBuilder } from 'knex';
-import { Accountability } from '../types/accountability';
+import { AbstractServiceOptions, Accountability, Collection } from '../types';
+import Knex from 'knex';
+import ItemsService from '../services/items';
+import FieldsService from '../services/fields';
+import { omit } from 'lodash';
 
-/**
- * @TODO turn this into a class
- */
+export default class CollectionsService extends ItemsService {
+	knex: Knex;
+	accountability: Accountability | null;
+	itemsService: ItemsService;
+	fieldsService: FieldsService;
 
-/** @Todo properly type this */
-export const create = async (payload: any, accountability?: Accountability) => {
-	const itemsService = new ItemsService('directus_collections', { accountability });
+	constructor(options?: AbstractServiceOptions) {
+		super('directus_collections', options);
 
-	await database.schema.createTable(payload.collection, (table) => {
-		if (payload.note) {
-			table.comment(payload.note);
+		this.knex = options?.knex || database;
+		this.accountability = options?.accountability || null;
+		this.itemsService = new ItemsService('directus_collections', options);
+		this.fieldsService = new FieldsService(options);
+	}
+
+	create(data: Partial<Collection>[]): Promise<string[]>;
+	create(data: Partial<Collection>): Promise<string>;
+	async create(data: Partial<Collection> | Partial<Collection>[]): Promise<string | string[]> {
+		const payloads = Array.isArray(data) ? data : [data];
+
+		// We'll create the fields separately. We don't want them to be inserted relationally
+		const payloadsWithoutFields = payloads.map((payload) => omit(payload, 'fields'));
+
+		await this.itemsService.create(payloadsWithoutFields);
+
+		for (const payload of payloads) {
+			// @TODO add basic validation to ensure all used fields are provided before attempting to save
+			await this.knex.schema.createTable(payload.collection!, async (table) => {
+				for (const field of payload.fields!) {
+					await this.fieldsService.createField(payload.collection!, field, table);
+				}
+			});
 		}
 
-		/** @todo move this into fields service */
-
-		/** @todo adhere to new nested structure system vs database */
-
-		payload.fields?.forEach((field: any) => {
-			let column: ColumnBuilder;
-
-			if (field.auto_increment) {
-				column = table.increments(field.field);
-			} else {
-				const datatype = field.length
-					? `${field.datatype}(${field.length})`
-					: field.datatype;
-				column = table.specificType(field.field, datatype);
-
-				// increments() also sets primary key
-				if (field.primary_key) {
-					column.primary();
-				}
-			}
-
-			if (field.note) {
-				column.comment(field.note);
-			}
-		});
-	});
-
-	const primaryKey = await itemsService.create({
-		collection: payload.collection,
-		hidden: payload.hidden || false,
-		singleton: payload.singleton || false,
-		icon: payload.icon || null,
-		note: payload.note || null,
-		translation: payload.translation || null,
-	});
+		const collectionNames = payloads.map((payload) => payload.collection!);
+		return Array.isArray(data) ? collectionNames : collectionNames[0];
+	}
 
 	/**
-	 * @TODO make this flexible and based on payload
+	 * @todo
+	 * update w/ nested fields
 	 */
-	await database('directus_fields').insert(
-		payload.fields.map((field: any) => ({
-			collection: payload.collection,
-			field: field.field,
-			locked: false,
-			required: false,
-			readonly: false,
-			hidden_detail: false,
-			hidden_browse: false,
-		}))
-	);
 
-	return await itemsService.readByKey(primaryKey);
-};
+	delete(collection: string): Promise<string>;
+	delete(collections: string[]): Promise<string[]>;
+	async delete(collection: string | string[]): Promise<string | string[]> {
+		const collections = Array.isArray(collection) ? collection : [collection];
 
-export const readAll = async (query: Query, accountability?: Accountability) => {
-	const itemsService = new ItemsService('directus_collections', { accountability });
+		/**
+		 * @todo check permissions manually
+		 * this.itemsService.delete does the permissions check, but we have to delete the records from fields/relations first
+		 * to adhere to the foreign key constraints
+		 */
 
-	const [tables, collections] = await Promise.all([
-		schemaInspector.tableInfo(),
-		itemsService.readByQuery(query),
-	]);
-
-	const data = tables.map((table) => {
-		const collectionInfo = collections.find((collection) => {
-			return collection.collection === table.name;
-		});
-
-		return {
-			collection: table.name,
-			note: table.comment,
-			hidden: collectionInfo?.hidden || false,
-			singleton: collectionInfo?.singleton || false,
-			icon: collectionInfo?.icon || null,
-			translation: collectionInfo?.translation || null,
-		};
-	});
-
-	return data;
-};
-
-export const readOne = async (
-	collection: string,
-	query: Query,
-	accountability?: Accountability
-) => {
-	const itemsService = new ItemsService('directus_collections', { accountability });
-
-	const [table, collectionInfo] = await Promise.all([
-		schemaInspector.tableInfo(collection),
-		itemsService.readByQuery(query),
-	]);
-
-	return {
-		collection: table.name,
-		note: table.comment,
-		hidden: collectionInfo[0]?.hidden || false,
-		singleton: collectionInfo[0]?.singleton || false,
-		icon: collectionInfo[0]?.icon || null,
-		translation: collectionInfo[0]?.translation || null,
-	};
-};
-
-export const deleteCollection = async (collection: string, accountability?: Accountability) => {
-	const itemsService = new ItemsService('directus_collections', { accountability });
-
-	await Promise.all([
-		database.schema.dropTable(collection),
-		itemsService.delete(collection),
-		database.delete().from('directus_fields').where({ collection }),
-		database
+		await this.knex('directus_fields').delete().whereIn('collection', collections);
+		await this.knex('directus_relations')
 			.delete()
-			.from('directus_relations')
-			.where({ many_collection: collection })
-			.orWhere({ one_collection: collection }),
-	]);
-};
+			.whereIn('many_collection', collections)
+			.orWhereIn('one_collection', collections);
+
+		await this.itemsService.delete(collection as any);
+
+		for (const collectionName of collections) {
+			await this.knex.schema.dropTable(collectionName);
+		}
+
+		return collection;
+	}
+}

api/src/services/fields.ts

@@ -7,7 +7,7 @@ import { ColumnBuilder } from 'knex';
 import getLocalType from '../utils/get-local-type';
 import { types } from '../types';
 import { FieldNotFoundException } from '../exceptions';
-import Knex from 'knex';
+import Knex, { CreateTableBuilder } from 'knex';
 
 type RawField = Partial<Field> & { field: string; type: typeof types[number] };
 
@@ -88,53 +88,65 @@ export default class FieldsService {
 	async createField(
 		collection: string,
 		field: Partial<Field> & { field: string; type: typeof types[number] },
-		accountability?: Accountability
+		table?: CreateTableBuilder // allows collection creation to
 	) {
-		const itemsService = new ItemsService('directus_fields', { accountability });
-
 		/**
 		 * @todo
 		 * Check if table / directus_fields row already exists
 		 */
 
 		if (field.database) {
-			await database.schema.alterTable(collection, (table) => {
-				let column: ColumnBuilder;
-
-				if (!field.database) return;
-
-				if (field.type === 'string') {
-					column = table.string(
-						field.field,
-						field.database.max_length !== null ? field.database.max_length : undefined
-					);
-				} else if (['float', 'decimal'].includes(field.type)) {
-					const type = field.type as 'float' | 'decimal';
-					/** @todo add precision and scale support */
-					column = table[type](field.field /* precision, scale */);
-				} else {
-					column = table[field.type](field.field);
-				}
-
-				if (field.database.default_value) {
-					column.defaultTo(field.database.default_value);
-				}
-
-				if (field.database.is_nullable && field.database.is_nullable === true) {
-					column.nullable();
-				} else {
-					column.notNullable();
-				}
-			});
+			if (table) {
+				addColumnToTable(table);
+			} else {
+				await database.schema.alterTable(collection, (table) => {
+					addColumnToTable(table);
+				});
+			}
 		}
 
 		if (field.system) {
-			await itemsService.create({
+			await this.service.create({
 				...field.system,
 				collection: collection,
 				field: field.field,
 			});
 		}
+
+		function addColumnToTable(table: CreateTableBuilder) {
+			let column: ColumnBuilder;
+
+			if (!field.database) return;
+
+			if (field.database.has_auto_increment) {
+				column = table.increments(field.field);
+			} else if (field.type === 'string') {
+				column = table.string(
+					field.field,
+					field.database.max_length !== null ? field.database.max_length : undefined
+				);
+			} else if (['float', 'decimal'].includes(field.type)) {
+				const type = field.type as 'float' | 'decimal';
+				/** @todo add precision and scale support */
+				column = table[type](field.field /* precision, scale */);
+			} else {
+				column = table[field.type](field.field);
+			}
+
+			if (field.database.default_value) {
+				column.defaultTo(field.database.default_value);
+			}
+
+			if (field.database.is_nullable && field.database.is_nullable === true) {
+				column.nullable();
+			} else {
+				column.notNullable();
+			}
+
+			if (field.database.is_primary_key) {
+				column.primary();
+			}
+		}
 	}
 
 	/** @todo research how to make this happen in SQLite / Redshift */

api/src/services/items.ts

@@ -79,7 +79,7 @@ export default class ItemsService implements AbstractService {
 			const primaryKeys: PrimaryKey[] = await trx
 				.insert(payloadsWithoutAliases)
 				.into(this.collection)
-				.returning('id');
+				.returning(primaryKeyField);
 
 			if (this.accountability) {
 				const activityRecords = primaryKeys.map((key) => ({

api/src/types/collection.ts

@@ -1,3 +1,5 @@
+import { Field } from './field';
+
 export type Collection = {
 	collection: string;
 	note: string | null;
@@ -5,4 +7,5 @@ export type Collection = {
 	single: boolean;
 	icon: string | null;
 	translation: Record<string, string>;
+	fields?: Field[];
 };