ronantakizawa/codereview-bench huggingface dataset:
Repository: directus/directus
File: api/src/services/comments.ts (TypeScript)

Code before:
					knex: this.knex,
				},
			);
		}

		const usersRegExp = new RegExp(/@[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}/gi);

		const mentions = uniq(data['comment'].match(usersRegExp) ?? []);

		const sender = await this.usersService.readOne(this.accountability!.user!, {
			fields: ['id', 'first_name', 'last_name', 'email'],
		});

		for (const mention of mentions) {
			const userID = mention.substring(1);

			const user = await this.usersService.readOne(userID, {
				fields: ['id', 'first_name', 'last_name', 'email', 'role.id'],
			});

			const accountability: Accountability = {
				user: userID,
				role: user['role']?.id ?? null,
				admin: false,
				app: false,
				roles: await fetchRolesTree(user['role']?.id, this.knex),
				ip: null,
			};

			const userGlobalAccess = await fetchGlobalAccess(accountability, this.knex);

			accountability.admin = userGlobalAccess.admin;
			accountability.app = userGlobalAccess.app;

			const usersService = new UsersService({ schema: this.schema, accountability });

			try {
				await validateAccess(
					{
						accountability,
						action: 'read',
						collection: data['collection'],
						primaryKeys: [data['item']],
					},
					{
						schema: this.schema,
						knex: this.knex,
					},
				);

				const templateData = await usersService.readByQuery({

Review comment (suggestion):
This function expects either a string or null, passing undefined (e.g. no role for the user) results in an error
```suggestion
				roles: await fetchRolesTree(user['role']?.id ?? null, this.knex),
```

Code after:
					knex: this.knex,
				},
			);
		}

		const usersRegExp = new RegExp(/@[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}/gi);

		const mentions = uniq(data['comment'].match(usersRegExp) ?? []);

		const sender = await this.usersService.readOne(this.accountability!.user!, {
			fields: ['id', 'first_name', 'last_name', 'email'],
		});

		for (const mention of mentions) {
			const userID = mention.substring(1);

			const user = await this.usersService.readOne(userID, {
				fields: ['id', 'first_name', 'last_name', 'email', 'role.id'],
			});

			const accountability: Accountability = {
				user: userID,
				role: user['role']?.id ?? null,
				admin: false,
				app: false,
				roles: await fetchRolesTree(user['role']?.id ?? null, this.knex),
				ip: null,
			};

			const userGlobalAccess = await fetchGlobalAccess(accountability, this.knex);

			accountability.admin = userGlobalAccess.admin;
			accountability.app = userGlobalAccess.app;

			const usersService = new UsersService({ schema: this.schema, accountability });

			try {
				await validateAccess(
					{
						accountability,
						action: 'read',
						collection: data['collection'],
						primaryKeys: [data['item']],
					},
					{
						schema: this.schema,
						knex: this.knex,
					},
				);

				const templateData = await usersService.readByQuery({
Repository: directus/directus
File: packages/storage-driver-supabase/src/index.ts (TypeScript)

Code before:
import type { ChunkedUploadContext, ReadOptions, TusDriver } from '@directus/storage';
import { normalizePath } from '@directus/utils';
import { StorageClient } from '@supabase/storage-js';
import * as tus from 'tus-js-client';
import { join } from 'node:path';
import { Readable } from 'node:stream';
import type { RequestInit } from 'undici';
import { fetch } from 'undici';

const DEFAULT_CHUNK_SIZE = 10_000_000;

export type DriverSupabaseConfig = {
	bucket: string;
	serviceRole: string;
	projectId?: string;
	/** Allows a custom Supabase endpoint for self-hosting */
	endpoint?: string;
	root?: string;
	tus?: {
		enabled: boolean;
		chunkSize?: number;
	};
};

export class DriverSupabase implements TusDriver {
	private config: DriverSupabaseConfig & { root: string };
	private client: StorageClient;
	private bucket: ReturnType<StorageClient['from']>;

	// TUS specific members
	private readonly preferredChunkSize: number;

	constructor(config: DriverSupabaseConfig) {
		this.config = {
			...config,
			root: normalizePath(config.root ?? '', { removeLeading: true }),
		};

		this.preferredChunkSize = this.config.tus?.chunkSize ?? DEFAULT_CHUNK_SIZE;

		this.client = this.getClient();
		this.bucket = this.getBucket();
	}

	private get endpoint() {

Review comment (suggestion):
I think we can skip adding this as it is unused. Similarly, it was not added for the S3 driver.

```suggestion
```

Code after:
import { DEFAULT_CHUNK_SIZE } from '@directus/constants';
import type { ChunkedUploadContext, ReadOptions, TusDriver } from '@directus/storage';
import { normalizePath } from '@directus/utils';
import { StorageClient } from '@supabase/storage-js';
import { join } from 'node:path';
import { Readable } from 'node:stream';
import * as tus from 'tus-js-client';
import type { RequestInit } from 'undici';
import { fetch } from 'undici';

export type DriverSupabaseConfig = {
	bucket: string;
	serviceRole: string;
	projectId?: string;
	/** Allows a custom Supabase endpoint for self-hosting */
	endpoint?: string;
	root?: string;
	tus?: {
		chunkSize?: number;
	};
};

export class DriverSupabase implements TusDriver {
	private config: DriverSupabaseConfig & { root: string };
	private client: StorageClient;
	private bucket: ReturnType<StorageClient['from']>;

	// TUS specific members
	private readonly preferredChunkSize: number;

	constructor(config: DriverSupabaseConfig) {
		this.config = {
			...config,
			root: normalizePath(config.root ?? '', { removeLeading: true }),
		};

		this.preferredChunkSize = this.config.tus?.chunkSize ?? DEFAULT_CHUNK_SIZE;

		this.client = this.getClient();
		this.bucket = this.getBucket();
	}

	private get endpoint() {
		return this.config.endpoint ?? `https://${this.config.projectId}.supabase.co/storage/v1`;
	}
Repository: directus/directus
File: packages/storage-driver-s3/src/index.ts (TypeScript)

Code before:

					const isDir = object.Key.endsWith('/');

					if (isDir) continue;

					yield object.Key.substring(this.root.length);
				}
			}
		} while (continuationToken);
	}

	// TUS implementation based on https://github.com/tus/tus-node-server

	get tusExtensions() {
		return ['creation', 'termination', 'expiration'];
	}

	async createChunkedUpload(filepath: string, context: ChunkedUploadContext): Promise<ChunkedUploadContext> {
		const command = new CreateMultipartUploadCommand({
			Bucket: this.config.bucket,
			Key: this.fullPath(filepath),
			Metadata: { 'tus-version': TUS_RESUMABLE },
			...(context.metadata?.['contentType']
				? {
						ContentType: context.metadata['contentType'],
					}
				: {}),
			...(context.metadata?.['cacheControl']
				? {
						CacheControl: context.metadata['cacheControl'],
					}
				: {}),
		});

		const res = await this.client.send(command);

		context.metadata!['upload-id'] = res.UploadId!;

		return context;
	}

	async deleteChunkedUpload(filepath: string, context: ChunkedUploadContext): Promise<void> {
		const key = this.fullPath(filepath);

		try {
			// @ts-expect-error
			const { 'upload-id': uploadId } = context.metadata;

			if (uploadId) {
				await this.client.send(
					new AbortMultipartUploadCommand({

Review comment (suggestion):
My VSCode is formatting this as tabs instead of spaces.
But why we have spaces in first place? I am rendering space characters and I only see tabs to ident, not spaces 🤔

Code after:

					const isDir = object.Key.endsWith('/');

					if (isDir) continue;

					yield object.Key.substring(this.root.length);
				}
			}
		} while (continuationToken);
	}

	// TUS implementation based on https://github.com/tus/tus-node-server

	get tusExtensions() {
		return ['creation', 'termination', 'expiration'];
	}

	async createChunkedUpload(filepath: string, context: ChunkedUploadContext): Promise<ChunkedUploadContext> {
		const command = new CreateMultipartUploadCommand({
			Bucket: this.config.bucket,
			Key: this.fullPath(filepath),
			Metadata: { 'tus-version': TUS_RESUMABLE },
			...(context.metadata?.['contentType']
				? {
						ContentType: context.metadata['contentType'],
				  }
				: {}),
			...(context.metadata?.['cacheControl']
				? {
						CacheControl: context.metadata['cacheControl'],
				  }
				: {}),
		});

		const res = await this.client.send(command);

		context.metadata!['upload-id'] = res.UploadId!;

		return context;
	}

	async deleteChunkedUpload(filepath: string, context: ChunkedUploadContext): Promise<void> {
		const key = this.fullPath(filepath);

		try {
			// @ts-expect-error
			const { 'upload-id': uploadId } = context.metadata;

			if (uploadId) {
				await this.client.send(
					new AbortMultipartUploadCommand({
Repository: directus/directus
File: app/src/stores/files.ts (TypeScript)

Code before:
import { defineStore } from 'pinia';
import { ref } from 'vue';

export const useFilesStore = defineStore('filesStore', () => {
	const uploading = ref(false);

	return {
		uploading,
	};
});


Review comment (suggestion):
When we're relying on a global true/false state, it'll mean that different consumers of this store can set it to false even though other processes are still uploading.

Instead, the internal state of the files store should be an array of upload IDs. We can export a `startUpload` function that itself returns the corresponding stop function. That way the upload component(s) can consume it as:

```js
const filesStore = useFilesStore();

const done = filesStore.startUpload();

// Do some async work to upload the file(s)

done();
```

Internally, this store could look something like (pseudo code):

```js
const active = ref([]);
const uploading = computed(() => active.value.length > 0);

const start = () => {
  const id = genId();
  active.value.push(id);

  return () => {
    active.value = active.value.filter((act) => act !== id));
  }
}

return { uploading, start };
```

Code after:
import { defineStore } from 'pinia';
import { computed, reactive } from 'vue';
import { nanoid } from 'nanoid';

type Upload = {
	numberOfFiles: number;
	progress: number;
	done: number; // number of files completed
};

export const useFilesStore = defineStore('filesStore', () => {
	// Each entry represents an upload, which can contain one or multiple uploads
	const activeUploads = reactive(new Map<string, Upload>());
	const isAnyUploadActive = computed(() => activeUploads.size > 0);

	return {
		isAnyUploadActive,
		upload,
	};

	function upload() {
		const id = nanoid();

		const uploading = computed(() => activeUploads.has(id));
		const numberOfFiles = computed(() => activeUploads.get(id)?.numberOfFiles ?? 0);

		const progress = computed({
			get() {
				return activeUploads.get(id)?.progress ?? 0;
			},
Repository: directus/directus
File: app/src/interfaces/input-rich-text-html/useImage.ts (TypeScript)

Code before:
					previewUrl: replaceUrlAccessToken(imageUrl, imageToken.value),
				};
			} else {
				imageSelection.value = null;
			}
		},
		onSetup: (buttonApi: any) => {
			const onImageNodeSelect = (eventApi: any) => {
				buttonApi.setActive(eventApi.element.tagName === 'IMG');
			};

			editor.value.on('NodeChange', onImageNodeSelect);

			return function () {
				editor.value.off('NodeChange', onImageNodeSelect);
			};
		},
	};

	return { imageDrawerOpen, imageSelection, closeImageDrawer, onImageSelect, saveImage, imageButton };

	function closeImageDrawer() {
		imageSelection.value = null;
		imageDrawerOpen.value = false;
	}

	function onImageSelect(image: File) {
		const fileExtension = image.filename_disk.split('.').pop();

		const assetUrl = getPublicURL() + 'assets/' + image.id + '.' + fileExtension;

		imageSelection.value = {
			imageUrl: replaceUrlAccessToken(assetUrl, imageToken.value),
			alt: image.title!,
			lazy: false,
			width: image.width,
			height: image.height,
			previewUrl: replaceUrlAccessToken(assetUrl, imageToken.value),
		};
	}

	function saveImage() {
		editor.value.fire('focus');

		const img = imageSelection.value;
		if (img === null) return;

		const queries: Record<string, any> = {};
		const newURL = new URL(img.imageUrl, 'file://');

		newURL.searchParams.delete('width');

Review comment (suggestion):
Since `readableMimeType` is unused, let's remove it from the imports.

Code after:
					previewUrl: replaceUrlAccessToken(imageUrl, imageToken.value),
				};
			} else {
				imageSelection.value = null;
			}
		},
		onSetup: (buttonApi: any) => {
			const onImageNodeSelect = (eventApi: any) => {
				buttonApi.setActive(eventApi.element.tagName === 'IMG');
			};

			editor.value.on('NodeChange', onImageNodeSelect);

			return function () {
				editor.value.off('NodeChange', onImageNodeSelect);
			};
		},
	};

	return { imageDrawerOpen, imageSelection, closeImageDrawer, onImageSelect, saveImage, imageButton };

	function closeImageDrawer() {
		imageSelection.value = null;
		imageDrawerOpen.value = false;
	}

	function onImageSelect(image: File) {
		const filenameDiskExtension = image.filename_disk.includes('.') ? image.filename_disk.split('.').pop() : undefined;

		const fileTypeExtension = image.type
			? readableMimeType(image.type, true)
			: readableMimeType(mime.getType(image.filename_download) as string, true);

		const fileExtension = filenameDiskExtension ?? fileTypeExtension;

		const assetUrl = getPublicURL() + 'assets/' + image.id + '.' + fileExtension;

		imageSelection.value = {
			imageUrl: replaceUrlAccessToken(assetUrl, imageToken.value),
			alt: image.title!,
			lazy: false,
			width: image.width,
			height: image.height,
			previewUrl: replaceUrlAccessToken(assetUrl, imageToken.value),
		};
	}

	function saveImage() {
		editor.value.fire('focus');

		const img = imageSelection.value;
Repository: directus/directus
File: api/src/database/migrations/20240924B-populate-versioning-deltas.ts (TypeScript)

Code before:
import type { Knex } from 'knex';
import { assign } from 'lodash-es';

export async function up(knex: Knex): Promise<void> {
	const rowsLimit = 50;
	let hasMore = true;

	while (hasMore) {
		const missingDeltaVersions = await knex.select('id').from('directus_versions').whereNull('delta').limit(rowsLimit);

		if (missingDeltaVersions.length === 0) {
			hasMore = false;
			break;
		}

		await knex.transaction(async (trx) => {
			for (const missingDeltaVersion of missingDeltaVersions) {
				const revisions = await trx
					.select('delta')
					.from('directus_revisions')
					.where('version', '=', missingDeltaVersion.id)
					.orderBy('id');

				const deltas = revisions.map((revision) =>
					revision.delta === 'string' ? parseJSON(revision.delta) : revision.delta ?? {},
				);

				const consolidatedDelta = assign({}, ...deltas);

				await trx('directus_versions')
					.update({
						delta: JSON.stringify(consolidatedDelta),
					})
					.where('id', '=', missingDeltaVersion.id);
			}
		});
	}
}

export async function down(): Promise<void> {
	// No down migration required
}


Review comment (suggestion):
```suggestion
				const deltas = revisions.map((revision) =>
					typeof revision.delta === 'string' ? parseJSON(revision.delta) : revision.delta ?? {},
				);
```

Code after:
import type { Knex } from 'knex';
import { assign } from 'lodash-es';

export async function up(knex: Knex): Promise<void> {
	const rowsLimit = 50;
	let hasMore = true;

	while (hasMore) {
		const missingDeltaVersions = await knex.select('id').from('directus_versions').whereNull('delta').limit(rowsLimit);

		if (missingDeltaVersions.length === 0) {
			hasMore = false;
			break;
		}

		await knex.transaction(async (trx) => {
			for (const missingDeltaVersion of missingDeltaVersions) {
				const revisions = await trx
					.select('delta')
					.from('directus_revisions')
					.where('version', '=', missingDeltaVersion.id)
					.orderBy('id');

				const deltas = revisions.map((revision) =>
					typeof revision.delta === 'string' ? parseJSON(revision.delta) : revision.delta ?? {},
				);

				const consolidatedDelta = assign({}, ...deltas);

				await trx('directus_versions')
					.update({
						delta: JSON.stringify(consolidatedDelta),
					})
					.where('id', '=', missingDeltaVersion.id);
			}
		});
	}
}

export async function down(): Promise<void> {
	// No down migration required
}
Repository: directus/directus
File: app/src/interfaces/translations/translations.vue (Vue)

Code before:
		defaultOpenSplitView?: boolean;
		userLanguage?: boolean;
		value: (number | string | Record<string, any>)[] | Record<string, any> | null;
		autofocus?: boolean;
		disabled?: boolean;
	}>(),
	{
		languageField: null,
		languageDirectionField: 'direction',
		value: () => [],
		autofocus: false,
		disabled: false,
		defaultLanguage: null,
		defaultOpenSplitView: false,
		userLanguage: false,
	},
);

const emit = defineEmits(['input']);

const value = computed({
	get: () => props.value ?? [],
	set: (val) => {
		emit('input', val);
	},
}) as WritableComputedRef<(number | string | Record<string, any>)[] | Record<string, any>>;

const { collection, field, primaryKey, defaultLanguage, userLanguage } = toRefs(props);
const { relationInfo } = useRelationM2M(collection, field);
const { t, locale } = useI18n();

const fieldsStore = useFieldsStore();

const { width } = useWindowSize();

const splitView = ref(props.defaultOpenSplitView);
const firstLang = ref<string>();
const secondLang = ref<string>();

watch(splitView, (splitViewEnabled) => {
	const lang = languageOptions.value;

	if (splitViewEnabled) {
		if (secondLang.value === firstLang.value || !secondLang.value) {
			const alternativeLang = lang.find((l) => l.value !== firstLang.value);
			secondLang.value = alternativeLang?.value ?? lang[0]?.value;
		}
	}
});

const { languageOptions } = useLanguages();

Review comment (suggestion):
I don't think this explicit type assertion is necessary here as it's inferred from the addition of the setter 🤔 

Code after:
		defaultOpenSplitView?: boolean;
		userLanguage?: boolean;
		value: (number | string | Record<string, any>)[] | Record<string, any> | null;
		autofocus?: boolean;
		disabled?: boolean;
	}>(),
	{
		languageField: null,
		languageDirectionField: 'direction',
		value: () => [],
		autofocus: false,
		disabled: false,
		defaultLanguage: null,
		defaultOpenSplitView: false,
		userLanguage: false,
	},
);

const emit = defineEmits(['input']);

const value = computed({
	get: () => props.value ?? [],
	set: (val) => {
		emit('input', val);
	},
});

const { collection, field, primaryKey, defaultLanguage, userLanguage } = toRefs(props);
const { relationInfo } = useRelationM2M(collection, field);
const { t, locale } = useI18n();

const fieldsStore = useFieldsStore();

const { width } = useWindowSize();

const splitView = ref(props.defaultOpenSplitView);
const firstLang = ref<string>();
const secondLang = ref<string>();

watch(splitView, (splitViewEnabled) => {
	if (splitViewEnabled) {
		const lang = languageOptions.value;
		const alternativeLang = lang.find((l) => l.value !== firstLang.value);
		secondLang.value = alternativeLang?.value ?? lang[0]?.value;
	}
});

const { languageOptions } = useLanguages();

const fields = computed(() => {
	if (!relationInfo.value) return [];
Repository: directus/directus
File: app/src/interfaces/translations/translations.vue (Vue)

Code before:
	get: () => props.value ?? [],
	set: (val) => {
		emit('input', val);
	},
}) as WritableComputedRef<(number | string | Record<string, any>)[] | Record<string, any>>;

const { collection, field, primaryKey, defaultLanguage, userLanguage } = toRefs(props);
const { relationInfo } = useRelationM2M(collection, field);
const { t, locale } = useI18n();

const fieldsStore = useFieldsStore();

const { width } = useWindowSize();

const splitView = ref(props.defaultOpenSplitView);
const firstLang = ref<string>();
const secondLang = ref<string>();

watch(splitView, (splitViewEnabled) => {
	const lang = languageOptions.value;

	if (splitViewEnabled) {
		if (secondLang.value === firstLang.value || !secondLang.value) {
			const alternativeLang = lang.find((l) => l.value !== firstLang.value);
			secondLang.value = alternativeLang?.value ?? lang[0]?.value;
		}
	}
});

const { languageOptions } = useLanguages();

const fields = computed(() => {
	if (!relationInfo.value) return [];
	return fieldsStore.getFieldsForCollection(relationInfo.value.junctionCollection.collection);
});

const query = ref<RelationQueryMultiple>({
	fields: ['*'],
	limit: -1,
	page: 1,
});

const { create, update, displayItems, loading, fetchedItems, getItemEdits } = useRelationMultiple(
	value,
	query,
	relationInfo,
	primaryKey,
);

const firstItem = computed(() => {
	const item = getItemWithLang(displayItems.value, firstLang.value);

Review comment (suggestion):
I don't think this is necessary as your assertion below already ensures that `secondLang.value` is set to `alternativeLang.value` so I think this is just performing that check and reassignment a second time for no reason

Code after:
	get: () => props.value ?? [],
	set: (val) => {
		emit('input', val);
	},
});

const { collection, field, primaryKey, defaultLanguage, userLanguage } = toRefs(props);
const { relationInfo } = useRelationM2M(collection, field);
const { t, locale } = useI18n();

const fieldsStore = useFieldsStore();

const { width } = useWindowSize();

const splitView = ref(props.defaultOpenSplitView);
const firstLang = ref<string>();
const secondLang = ref<string>();

watch(splitView, (splitViewEnabled) => {
	if (splitViewEnabled) {
		const lang = languageOptions.value;
		const alternativeLang = lang.find((l) => l.value !== firstLang.value);
		secondLang.value = alternativeLang?.value ?? lang[0]?.value;
	}
});

const { languageOptions } = useLanguages();

const fields = computed(() => {
	if (!relationInfo.value) return [];
	return fieldsStore.getFieldsForCollection(relationInfo.value.junctionCollection.collection);
});

const query = ref<RelationQueryMultiple>({
	fields: ['*'],
	limit: -1,
	page: 1,
});

const { create, update, displayItems, loading, fetchedItems, getItemEdits } = useRelationMultiple(
	value,
	query,
	relationInfo,
	primaryKey,
);

const firstItem = computed(() => {
	const item = getItemWithLang(displayItems.value, firstLang.value);
	if (item === undefined) return undefined;

	return getItemEdits(item);