ronantakizawa/codereview-bench huggingface dataset:
Repository: directus/directus
File: sdk/src/rest/commands/create/files.ts (TypeScript)

Code before:
export const uploadFiles =
	<Schema, const TQuery extends Query<Schema, DirectusFile<Schema>>>(
		data: FormData,
		query?: TQuery,
	): RestCommand<CreateFileOutput<Schema, TQuery>, Schema> =>
	() => ({
		path: '/files',
		method: 'POST',
		body: data,
		params: query ?? {},
		headers: { 'Content-Type': 'multipart/form-data' },
	});

/**
 * Import a file from the web
 *
 * @param url The url to import the file from
 * @param data Formdata object
 * @param query The query parameters
 *
 * @returns Returns the file object for the imported file.
 */
export const importFile =
	<Schema, TQuery extends Query<Schema, DirectusFile<Schema>>>(
		url: string,
		data: NestedPartial<DirectusFile<Schema>>  | {} = {},
		query?: TQuery,
	): RestCommand<CreateFileOutput<Schema, TQuery>, Schema> =>
	() => ({
		path: '/files/import',
		method: 'POST',
		body: JSON.stringify({ url, data }),
		params: query ?? {},
	});


Review comment (suggestion):
```suggestion
		data: NestedPartial<DirectusFile<Schema>> = {},
```
The `| {}` type here is not needed and somewhat misleading because an empty type `{}` behaves as `any` allowing for any type. The `NestedPartial` should make all properties optional allowing for an empty object
<img width="1567" height="177" alt="image" src="https://github.com/user-attachments/assets/66756073-de36-40b7-a4c6-7db41ca17a00" />


Code after:
export const uploadFiles =
	<Schema, const TQuery extends Query<Schema, DirectusFile<Schema>>>(
		data: FormData,
		query?: TQuery,
	): RestCommand<CreateFileOutput<Schema, TQuery>, Schema> =>
	() => ({
		path: '/files',
		method: 'POST',
		body: data,
		params: query ?? {},
		headers: { 'Content-Type': 'multipart/form-data' },
	});

/**
 * Import a file from the web
 *
 * @param url The url to import the file from
 * @param data Formdata object
 * @param query The query parameters
 *
 * @returns Returns the file object for the imported file.
 */
export const importFile =
	<Schema, TQuery extends Query<Schema, DirectusFile<Schema>>>(
		url: string,
		data?: NestedPartial<DirectusFile<Schema>>,
		query?: TQuery,
	): RestCommand<CreateFileOutput<Schema, TQuery>, Schema> =>
	() => ({
		path: '/files/import',
		method: 'POST',
		body: JSON.stringify({ url, data: data ?? {} }),
		params: query ?? {},
	});
Repository: directus/directus
File: sdk/src/rest/commands/update/singleton.ts (TypeScript)

Code before:
import type { ApplyQueryFields, CollectionType, NestedPartial, Query, SingletonCollections } from '../../../types/index.js';
import type { RestCommand } from '../../types.js';
import { throwIfCoreCollection, throwIfEmpty } from '../../utils/index.js';

export type UpdateSingletonOutput<
	Schema,
	Collection extends SingletonCollections<Schema>,
	TQuery extends Query<Schema, Schema[Collection]>,
> = ApplyQueryFields<Schema, CollectionType<Schema, Collection>, TQuery['fields']>;

/**
 * Update a singleton item
 *
 * @param collection The collection of the items
 * @param query The query parameters
 *
 * @returns An array of up to limit item objects. If no items are available, data will be an empty array.
 * @throws Will throw if collection is a core collection
 * @throws Will throw if collection is empty
 */
export const updateSingleton =
	<
		Schema,
		Collection extends SingletonCollections<Schema>,
		const TQuery extends Query<Schema, Schema[Collection]>,
		Item = Schema[Collection],

Review comment (suggestion):
```suggestion
import type {
	ApplyQueryFields,
	CollectionType,
	NestedPartial,
	Query,
	SingletonCollections,
} from '../../../types/index.js';
```
lazy prettier fix

Code after:
import type {
	ApplyQueryFields,
	CollectionType,
	NestedPartial,
	Query,
	SingletonCollections,
} from '../../../types/index.js';
import type { RestCommand } from '../../types.js';
import { throwIfCoreCollection, throwIfEmpty } from '../../utils/index.js';

export type UpdateSingletonOutput<
	Schema,
	Collection extends SingletonCollections<Schema>,
	TQuery extends Query<Schema, Schema[Collection]>,
> = ApplyQueryFields<Schema, CollectionType<Schema, Collection>, TQuery['fields']>;

/**
 * Update a singleton item
 *
 * @param collection The collection of the items
 * @param query The query parameters
 *
 * @returns An array of up to limit item objects. If no items are available, data will be an empty array.
 * @throws Will throw if collection is a core collection
 * @throws Will throw if collection is empty
 */
Repository: directus/directus
File: app/src/components/v-form/form-field-label.vue (Vue)

Code before:

	.required {
		--v-icon-color: var(--theme--primary);

		margin-inline-start: 3px;

		&.has-badge {
			margin-inline-end: 6px;
		}
	}

	.ctx-arrow {
		margin-block-start: -3px;
		color: var(--theme--foreground-subdued);
		opacity: 0;
		transition: opacity var(--fast) var(--transition);

		&.active {
			opacity: 1;
		}
	}

	.hidden-indicator {
		margin-inline-start: 4px;
		color: var(--theme--foreground-subdued);
		font-size: 0.875em;
	}

	&:focus-within,
	&:hover {
		.ctx-arrow {
			opacity: 1;
		}
	}

	.raw-editor-toggle {
		display: inline-flex;
		align-items: center;
		justify-content: center;
		block-size: 24px;
		inline-size: 24px;
		margin-block-start: -2px;
		margin-inline-start: 5px;
		color: var(--theme--foreground-subdued);
		transition: color var(--fast) var(--transition);

		&:hover {
			color: var(--theme--foreground);
		}

		&.active {

Review comment (suggestion):
Let’s use the same font-size according to the design and use em for spacing.

```suggestion
	.hidden-indicator {
		margin-inline-start: 0.25em;
		color: var(--theme--foreground-subdued);
	}
```

Code after:

	.required {
		--v-icon-color: var(--theme--primary);

		margin-inline-start: 3px;

		&.has-badge {
			margin-inline-end: 6px;
		}
	}

	.ctx-arrow {
		margin-block-start: -3px;
		color: var(--theme--foreground-subdued);
		opacity: 0;
		transition: opacity var(--fast) var(--transition);

		&.active {
			opacity: 1;
		}
	}

	.hidden-indicator {
		margin-inline-start: 0.25em;
		color: var(--theme--foreground-subdued);
	}

	&:focus-within,
	&:hover {
		.ctx-arrow {
			opacity: 1;
		}
	}

	.raw-editor-toggle {
		display: inline-flex;
		align-items: center;
		justify-content: center;
		block-size: 24px;
		inline-size: 24px;
		margin-block-start: -2px;
		margin-inline-start: 5px;
		color: var(--theme--foreground-subdued);
		transition: color var(--fast) var(--transition);

		&:hover {
			color: var(--theme--foreground);
		}

		&.active {
			color: var(--theme--primary);
Repository: directus/directus
File: sdk/src/rest/commands/update/fields.ts (TypeScript)

Code before:
	() => {
		throwIfEmpty(collection, 'Keys cannot be empty');
		throwIfEmpty(field, 'Field cannot be empty');

		return {
			path: `/fields/${collection}/${field}`,
			params: query ?? {},
			body: JSON.stringify(item),
			method: 'PATCH',
		};
	};

/**
 * Update multiple existing fields.
 * @param collection
 * @param items
 * @param query
 * @returns
 * @throws Will throw if collection is empty
 */
export const updateFields =
	<Schema, const TQuery extends Query<Schema, DirectusField<Schema>>>(
		collection: DirectusField<Schema>['collection'],
		items: NestedPartial<DirectusField<Schema>>[],
		query?: TQuery,
	): RestCommand<UpdateFieldOutput<Schema, TQuery>, Schema> =>
	() => {
		throwIfEmpty(collection, 'Keys cannot be empty');

		return {
			path: `/fields/${collection}`,
			params: query ?? {},
			body: JSON.stringify(items),
			method: 'PATCH',
		};
	};


Review comment (suggestion):
This is [a bulk endpoint](https://github.com/directus/directus/blob/453d2ba2b3cf193369c3a569fe800d29424e2bd3/api/src/controllers/fields.ts#L141)

```suggestion
	): RestCommand<UpdateFieldOutput<Schema, TQuery>[], Schema> =>
```

Code after:
	() => {
		throwIfEmpty(collection, 'Keys cannot be empty');
		throwIfEmpty(field, 'Field cannot be empty');

		return {
			path: `/fields/${collection}/${field}`,
			params: query ?? {},
			body: JSON.stringify(item),
			method: 'PATCH',
		};
	};

/**
 * Update multiple existing fields.
 * @param collection
 * @param items
 * @param query
 * @returns
 * @throws Will throw if collection is empty
 */
export const updateFields =
	<Schema, const TQuery extends Query<Schema, DirectusField<Schema>>>(
		collection: DirectusField<Schema>['collection'],
		items: NestedPartial<DirectusField<Schema>>[],
		query?: TQuery,
	): RestCommand<UpdateFieldOutput<Schema, TQuery>[], Schema> =>
	() => {
		throwIfEmpty(collection, 'Collection cannot be empty');

		return {
			path: `/fields/${collection}`,
			params: query ?? {},
			body: JSON.stringify(items),
			method: 'PATCH',
		};
	};
Repository: directus/directus
File: sdk/src/rest/commands/update/fields.ts (TypeScript)

Code before:
		throwIfEmpty(field, 'Field cannot be empty');

		return {
			path: `/fields/${collection}/${field}`,
			params: query ?? {},
			body: JSON.stringify(item),
			method: 'PATCH',
		};
	};

/**
 * Update multiple existing fields.
 * @param collection
 * @param items
 * @param query
 * @returns
 * @throws Will throw if collection is empty
 */
export const updateFields =
	<Schema, const TQuery extends Query<Schema, DirectusField<Schema>>>(
		collection: DirectusField<Schema>['collection'],
		items: NestedPartial<DirectusField<Schema>>[],
		query?: TQuery,
	): RestCommand<UpdateFieldOutput<Schema, TQuery>[], Schema> =>
	() => {
		throwIfEmpty(collection, 'Keys cannot be empty');

		return {
			path: `/fields/${collection}`,
			params: query ?? {},
			body: JSON.stringify(items),
			method: 'PATCH',
		};
	};


Review comment (suggestion):
```suggestion
		throwIfEmpty(collection, 'Collection cannot be empty');
```

Code after:
		throwIfEmpty(field, 'Field cannot be empty');

		return {
			path: `/fields/${collection}/${field}`,
			params: query ?? {},
			body: JSON.stringify(item),
			method: 'PATCH',
		};
	};

/**
 * Update multiple existing fields.
 * @param collection
 * @param items
 * @param query
 * @returns
 * @throws Will throw if collection is empty
 */
export const updateFields =
	<Schema, const TQuery extends Query<Schema, DirectusField<Schema>>>(
		collection: DirectusField<Schema>['collection'],
		items: NestedPartial<DirectusField<Schema>>[],
		query?: TQuery,
	): RestCommand<UpdateFieldOutput<Schema, TQuery>[], Schema> =>
	() => {
		throwIfEmpty(collection, 'Collection cannot be empty');

		return {
			path: `/fields/${collection}`,
			params: query ?? {},
			body: JSON.stringify(items),
			method: 'PATCH',
		};
	};
Repository: directus/directus
File: api/src/services/items.ts (TypeScript)

Code before:
			};

			if (opts.bypassEmitAction) {
				opts.bypassEmitAction(actionEvent);
			} else {
				emitter.emitAction(actionEvent.event, actionEvent.meta, actionEvent.context);
			}
		}

		return keys;
	}

	/**
	 * Read/treat collection as singleton.
	 */
	async readSingleton(query: Query, opts?: QueryOptions): Promise<Partial<Item>> {
		query = clone(query);

		query.limit = 1;

		let record;

		if (query.version) {
			const primaryKeyField = this.schema.collections[this.collection]!.primary;
			const key = (await this.knex.select(primaryKeyField).from(this.collection).limit(1).first())[primaryKeyField];
			record = await handleVersion(this, key, query, opts);
		} else {
			record = (await this.readByQuery(query, opts))[0];
		}

		if (!record) {
			let fields = Object.entries(this.schema.collections[this.collection]!.fields);
			const defaults: Record<string, any> = {};

			if (query.fields && query.fields.includes('*') === false) {
				fields = fields.filter(([name]) => {
					return query.fields!.includes(name);
				});
			}

			for (const [name, field] of fields) {
				if (this.schema.collections[this.collection]!.primary === name) {
					defaults[name] = null;
					continue;
				}

				if (field.defaultValue !== null) defaults[name] = field.defaultValue;
			}

			return defaults as Partial<Item>;
		}

Review comment (suggestion):
`.first()` can return undefined if the singleton has yet to be created and a version is attempted to be queried.

```suggestion
			const primaryKeyField = this.schema.collections[this.collection]!.primary;
			const queryKey = await this.knex.select(primaryKeyField).from(this.collection).limit(1).first();
			const key = queryKey?.[primaryKeyField];

			if (key) {
				record = await handleVersion(this, key, query, opts);
			}
```

Code after:
			};

			if (opts.bypassEmitAction) {
				opts.bypassEmitAction(actionEvent);
			} else {
				emitter.emitAction(actionEvent.event, actionEvent.meta, actionEvent.context);
			}
		}

		return keys;
	}

	/**
	 * Read/treat collection as singleton.
	 */
	async readSingleton(query: Query, opts?: QueryOptions): Promise<Partial<Item>> {
		query = clone(query);

		query.limit = 1;

		let record;

		if (query.version) {
			const primaryKeyField = this.schema.collections[this.collection]!.primary;
			const key = (await this.knex.select(primaryKeyField).from(this.collection).first())?.[primaryKeyField];

			if (key) {
				record = await handleVersion(this, key, query, opts);
			}
		} else {
			record = (await this.readByQuery(query, opts))[0];
		}

		if (!record) {
			let fields = Object.entries(this.schema.collections[this.collection]!.fields);
			const defaults: Record<string, any> = {};

			if (query.fields && query.fields.includes('*') === false) {
				fields = fields.filter(([name]) => {
					return query.fields!.includes(name);
				});
			}

			for (const [name, field] of fields) {
				if (this.schema.collections[this.collection]!.primary === name) {
					defaults[name] = null;
					continue;
				}

				if (field.defaultValue !== null) defaults[name] = field.defaultValue;
			}
Repository: directus/directus
File: app/src/components/v-upload.vue (Vue)

Code before:
const { uploading, progress, upload, onBrowseSelect, done, numberOfFiles } = useUpload();
const { onDragEnter, onDragLeave, onDrop, dragging } = useDragging();
const { url, isValidURL, loading: urlLoading, importFromURL } = useURLImport();
const { setSelection } = useSelection();
const activeDialog = ref<'choose' | 'url' | null>(null);
const input = ref<HTMLInputElement>();

onUnmounted(() => {
	uploadController?.abort();
});

function validFiles(files: FileList) {
	for (const file of files) {
		if (file.size === 0) {
			notificationsStore.add({
				title: t('invalid_file_type'),
				text: t('empty_file_error', { filename: file.name }),
				type: 'error',
				dialog: true,
			});

			return false;
		}

		if (props.accept) {
			const acceptTypes = props.accept.split(',').map((type) => type.trim());

			const isValidType = acceptTypes.some((acceptType) => {
				if (acceptType.endsWith('/*')) {
					const baseType = acceptType.slice(0, -2);

					return file.type.startsWith(baseType + '/');
				} else {
					return file.type === acceptType;
				}
			});

			if (!isValidType) {
				notificationsStore.add({
					title: t('invalid_file_type'),
					text: t('file_type_not_allowed', { type: file.type, accept: props.accept }),
					type: 'error',
					dialog: true,
				});

				return false;
			}
		}
	}

	return true;

Review comment (suggestion):
I'm wondering if we should be explicit and show an error when trying to upload a non-image as part of the set, rather than ignoring the non-images and continuing the upload for the others 🤔 

Code after:
	input: [files: null | File | File[]];
	start: [controller: UploadController];
}>();

const { t } = useI18n();
const notificationsStore = useNotificationsStore();

let uploadController: Upload | null = null;

const { uploading, progress, upload, onBrowseSelect, done, numberOfFiles } = useUpload();
const { onDragEnter, onDragLeave, onDrop, dragging } = useDragging();
const { url, isValidURL, loading: urlLoading, importFromURL } = useURLImport();
const { setSelection } = useSelection();
const activeDialog = ref<'choose' | 'url' | null>(null);
const input = ref<HTMLInputElement>();

onUnmounted(() => {
	uploadController?.abort();
});

function validFiles(files: FileList) {
	const typeErrors: string[] = [];
	const emptyErrors: string[] = [];

	for (const file of files) {
		if (file.size === 0) {
			emptyErrors.push(`"${file.name}"`);
			continue;
		}

		if (props.accept) {
			const acceptTypes = props.accept.split(',').map((type) => type.trim());

			const isValidType = acceptTypes.some((acceptType) => {
				if (acceptType.endsWith('/*')) {
					const baseType = acceptType.slice(0, -2);

					return file.type.startsWith(baseType + '/');
				} else {
					return file.type === acceptType;
				}
			});

			if (!isValidType) {
				typeErrors.push(`"${file.name}" (${file.type})`);
			}
		}
	}

	const totalErrors = typeErrors.length + emptyErrors.length;
Repository: directus/directus
File: api/src/auth/auth.ts (TypeScript)

Code before:
import type { SchemaOverview } from '@directus/types';
import type { Knex } from 'knex';
import { UsersService } from '../services/users.js';
import type { AuthDriverOptions, User } from '../types/index.js';
import { getSchema } from '../utils/get-schema.js';

export abstract class AuthDriver {
	knex: Knex;

	constructor(options: AuthDriverOptions, _config: Record<string, any>) {
		this.knex = options.knex;
	}

	protected async getCurrentSchema(): Promise<SchemaOverview> {
		return await getSchema();
	}

	protected async getUsersService(schema?: SchemaOverview): Promise<UsersService> {
		return new UsersService({
			knex: this.knex,
			schema: schema ?? (await this.getCurrentSchema()),
		});
	}

	/**
	 * Get user id for a given provider payload
	 *
	 * @param payload Any data that the user might've provided
	 * @throws InvalidCredentialsError
	 * @return User id of the identifier
	 */
	abstract getUserID(payload: Record<string, any>): Promise<string>;

	/**
	 * Verify user password
	 *
	 * @param user User information
	 * @param password User password
	 * @throws InvalidCredentialsError
	 */
	abstract verify(user: User, password?: string): Promise<void>;

	/**
	 * Check with the (external) provider if the user is allowed entry to Directus
	 *
	 * @param _user User information
	 * @param _payload Any data that the user might've provided
	 * @throws InvalidCredentialsError

Review comment (suggestion):
It feels a little bit like overkill to wrap the globally available `getSchema` function into a method of this class without extra logic 🤔 
```suggestion
	protected async getUsersService(schema?: SchemaOverview): Promise<UsersService> {
		return new UsersService({
			knex: this.knex,
			schema: schema ?? (await getSchema()),
		});
	}
```

Code after:
import type { SchemaOverview } from '@directus/types';
import type { Knex } from 'knex';
import { UsersService } from '../services/users.js';
import type { AuthDriverOptions, User } from '../types/index.js';

export abstract class AuthDriver {
	knex: Knex;

	constructor(options: AuthDriverOptions, _config: Record<string, any>) {
		this.knex = options.knex;
	}

	protected getUsersService(schema: SchemaOverview): UsersService {
		return new UsersService({
			knex: this.knex,
			schema,
		});
	}

	/**
	 * Get user id for a given provider payload
	 *
	 * @param payload Any data that the user might've provided
	 * @throws InvalidCredentialsError
	 * @return User id of the identifier
	 */
	abstract getUserID(payload: Record<string, any>): Promise<string>;

	/**
	 * Verify user password
	 *
	 * @param user User information
	 * @param password User password
	 * @throws InvalidCredentialsError
	 */
	abstract verify(user: User, password?: string): Promise<void>;

	/**
	 * Check with the (external) provider if the user is allowed entry to Directus
	 *
	 * @param _user User information
	 * @param _payload Any data that the user might've provided
	 * @throws InvalidCredentialsError
	 * @returns Data to be stored with the session
	 */
	async login(_user: User, _payload: Record<string, any>): Promise<void> {
		return;
	}
Repository: directus/directus
File: api/src/auth/drivers/ldap.ts (TypeScript)

Code before:
		}

		const userId = await this.fetchUserId(userInfo.dn);

		if (userId) {
			// Run hook so the end user has the chance to augment the
			// user that is about to be updated
			let emitPayload = {};

			// Only sync roles if the AD groups are configured
			if (groupDn) {
				emitPayload = {
					role: userRole?.id ?? defaultRoleId ?? null,
				};
			}

			if (syncUserInfo) {
				emitPayload = {
					...emitPayload,
					first_name: userInfo.firstName,
					last_name: userInfo.lastName,
					email: userInfo.email,
				};
			}

			const schema = await this.getCurrentSchema();

			const updatedUserPayload = await emitter.emitFilter(
				`auth.update`,
				emitPayload,
				{ identifier: userInfo.dn, provider: this.config['provider'], providerPayload: { userInfo, userRole } },
				{ database: getDatabase(), schema, accountability: null },
			);

			// Update user to update properties that might have changed
			const usersService = await this.getUsersService(schema);
			await usersService.updateOne(userId, updatedUserPayload);

			return userId;
		}

		if (!userInfo) {
			throw new InvalidCredentialsError();
		}

		const userPayload = {
			provider: this.config['provider'],
			first_name: userInfo.firstName,
			last_name: userInfo.lastName,
			email: userInfo.email,
			external_identifier: userInfo.dn,

Review comment (suggestion):
Same for the other usages but wont make a comment for each one 😄 
```suggestion
			const schema = await getSchema();
```

Code after:
			}
		}

		const userId = await this.fetchUserId(userInfo.dn);

		if (userId) {
			// Run hook so the end user has the chance to augment the
			// user that is about to be updated
			let emitPayload = {};

			// Only sync roles if the AD groups are configured
			if (groupDn) {
				emitPayload = {
					role: userRole?.id ?? defaultRoleId ?? null,
				};
			}

			if (syncUserInfo) {
				emitPayload = {
					...emitPayload,
					first_name: userInfo.firstName,
					last_name: userInfo.lastName,
					email: userInfo.email,
				};
			}

			const schema = await getSchema();

			const updatedUserPayload = await emitter.emitFilter(
				`auth.update`,
				emitPayload,
				{ identifier: userInfo.dn, provider: this.config['provider'], providerPayload: { userInfo, userRole } },
				{ database: getDatabase(), schema, accountability: null },
			);

			// Update user to update properties that might have changed
			const usersService = this.getUsersService(schema);
			await usersService.updateOne(userId, updatedUserPayload);

			return userId;
		}

		if (!userInfo) {
			throw new InvalidCredentialsError();
		}

		const userPayload = {
			provider: this.config['provider'],
			first_name: userInfo.firstName,
			last_name: userInfo.lastName,
			email: userInfo.email,