ronantakizawa/codereview-bench huggingface dataset:
Repository: directus/directus
File: app/src/components/v-form/v-form.vue (Vue)

Code before:
	[field: string]: any;
};

const props = withDefaults(
	defineProps<{
		collection?: string;
		fields?: Field[];
		initialValues?: FieldValues | null;
		modelValue?: FieldValues | null;
		loading?: boolean;
		batchMode?: boolean;
		primaryKey?: string | number;
		disabled?: boolean;
		validationErrors?: ValidationError[];
		autofocus?: boolean;
		group?: string | null;
		badge?: string;
		showValidationErrors?: boolean;
		showNoVisibleFields?: boolean;
		/* Enable the raw editor toggler on fields */
		rawEditorEnabled?: boolean;
		disabledMenuOptions?: MenuOptions[];
		direction?: string;
		showDivider?: boolean;
		inline?: boolean;
		currentVersion?: ContentVersion | null;
	}>(),
	{
		collection: undefined,
		fields: undefined,
		initialValues: null,
		modelValue: null,
		primaryKey: undefined,
		validationErrors: () => [],
		group: null,
		badge: undefined,
		showValidationErrors: true,
		showNoVisibleFields: true,
		direction: undefined,
		currentVersion: null,
	},
);

const { t } = useI18n();

const emit = defineEmits(['update:modelValue']);

const values = computed(() => {
	return Object.assign({}, cloneDeep(props.initialValues), cloneDeep(props.modelValue));
});


Review comment (suggestion):
See the other comment

```suggestion
		version?: ContentVersion | null;
```

Code after:
	[field: string]: any;
};

const props = withDefaults(
	defineProps<{
		collection?: string;
		fields?: Field[];
		initialValues?: FieldValues | null;
		modelValue?: FieldValues | null;
		loading?: boolean;
		batchMode?: boolean;
		primaryKey?: string | number;
		disabled?: boolean;
		validationErrors?: ValidationError[];
		autofocus?: boolean;
		group?: string | null;
		badge?: string;
		showValidationErrors?: boolean;
		showNoVisibleFields?: boolean;
		/* Enable the raw editor toggler on fields */
		rawEditorEnabled?: boolean;
		disabledMenuOptions?: MenuOptions[];
		direction?: string;
		showDivider?: boolean;
		inline?: boolean;
		version?: ContentVersion | null;
	}>(),
	{
		collection: undefined,
		fields: undefined,
		initialValues: null,
		modelValue: null,
		primaryKey: undefined,
		validationErrors: () => [],
		group: null,
		badge: undefined,
		showValidationErrors: true,
		showNoVisibleFields: true,
		direction: undefined,
		version: null,
	},
);

const { t } = useI18n();

const emit = defineEmits(['update:modelValue']);

const values = computed(() => {
	return Object.assign({}, cloneDeep(props.initialValues), cloneDeep(props.modelValue));
});
Repository: directus/directus
File: app/src/composables/use-versions.ts (TypeScript)

Code before:
		saveVersionLoading.value = true;

		if (!currentVersion.value) return;

		const payloadToValidate = mergeWith(
			{},
			defaultValues.value,
			item.value,
			edits.value,
			function (_from: any, to: any) {
				if (typeof to !== 'undefined') {
					return to;
				}
			},
		);

		const fields = pushGroupOptionsDown(fieldsWithPermissions.value);

		const errors = validateItem(payloadToValidate, fields, false, false, currentVersion.value);
		if (nestedValidationErrors.value?.length) errors.push(...nestedValidationErrors.value);

		if (errors.length > 0) {
			validationErrors.value = errors;
			saveVersionLoading.value = false;
			throw errors;
		}

		try {
			const response = await api.post(`/versions/${currentVersion.value.id}/save`, unref(edits));
			const savedData = response.data.data;

			// Update local item with the saved changes
			Object.assign(item.value, savedData);
			edits.value = {};

			return savedData;
		} catch (error) {
			saveVersionErrorHandler(error);
		} finally {
			saveVersionLoading.value = false;
		}
	}
}


Review comment (refactor):
This validation part is a duplicate of what we have in `app/src/composables/use-item.ts`. I think it is a good idea to extract this into a function/composable for reuse. But be careful that you don’t lose the reactivity of the passed Refs.

Also the `saveVersionErrorHandler` function is a duplicate of the file mentioned above. So it might make sense to have such a composable that returns `errors` and a `saveErrorHandler` function for instance to reuse it in both places.

Code after:
		if (!currentVersion.value) return;
		saveVersionLoading.value = true;
		validationErrors.value = [];

		const payloadToValidate = mergeWith(
			{},
			defaultValues.value,
			item.value,
			edits.value,
			function (_from: any, to: any) {
				if (typeof to !== 'undefined') {
					return to;
				}
			},
		);

		const fields = pushGroupOptionsDown(fieldsWithPermissions.value);

		const errors = validateItem(payloadToValidate, fields, false, false, currentVersion.value);
		if (nestedValidationErrors.value?.length) errors.push(...nestedValidationErrors.value);

		if (errors.length > 0) {
			validationErrors.value = errors;
			saveVersionLoading.value = false;
			throw errors;
		}

		try {
			const response = await api.post(`/versions/${currentVersion.value.id}/save`, unref(edits));
			const savedData = response.data.data;

			// Update local item with the saved changes
			Object.assign(item.value, savedData);
			edits.value = {};

			return savedData;
		} catch (error) {
			saveVersionErrorHandler(error);
		} finally {
			saveVersionLoading.value = false;
		}
	}
}
Repository: directus/directus
File: app/src/composables/use-versions.ts (TypeScript)

Code before:
		const currentVersionId = currentVersion.value.id;

		const versionToUpdate = versions.value.find((version) => version.id === currentVersionId);

		if (versionToUpdate) {
			versionToUpdate.key = updates.key;
			if ('name' in updates) versionToUpdate.name = updates.name ?? null;
			currentVersion.value = versionToUpdate;
		}
	}

	async function deleteVersion() {
		if (!currentVersion.value || !versions.value) return;

		const currentVersionId = currentVersion.value.id;

		const index = versions.value.findIndex((version) => version.id === currentVersionId);

		if (index !== undefined) {
			currentVersion.value = null;
			versions.value.splice(index, 1);
		}
	}

	async function saveVersion(edits: Ref<Record<string, any>>, item: Ref<Record<string, any>>) {
		saveVersionLoading.value = true;

		if (!currentVersion.value) return;

		const payloadToValidate = mergeWith(
			{},
			defaultValues.value,
			item.value,
			edits.value,
			function (_from: any, to: any) {
				if (typeof to !== 'undefined') {
					return to;
				}
			},
		);

		const fields = pushGroupOptionsDown(fieldsWithPermissions.value);

		const errors = validateItem(payloadToValidate, fields, false, false, currentVersion.value);
		if (nestedValidationErrors.value?.length) errors.push(...nestedValidationErrors.value);

		if (errors.length > 0) {
			validationErrors.value = errors;
			saveVersionLoading.value = false;
			throw errors;
		}

Review comment (suggestion):
If the `currentVersion.value` is `null` or `falsy` the `saveVersionLoading.value` will remain `true` … so I think it is a good idea to reverse the order of these lines again.

Code after:
		const currentVersionId = currentVersion.value.id;

		const versionToUpdate = versions.value.find((version) => version.id === currentVersionId);

		if (versionToUpdate) {
			versionToUpdate.key = updates.key;
			if ('name' in updates) versionToUpdate.name = updates.name ?? null;
			currentVersion.value = versionToUpdate;
		}
	}

	async function deleteVersion() {
		if (!currentVersion.value || !versions.value) return;

		const currentVersionId = currentVersion.value.id;

		const index = versions.value.findIndex((version) => version.id === currentVersionId);

		if (index !== undefined) {
			currentVersion.value = null;
			versions.value.splice(index, 1);
		}
	}

	async function saveVersion(edits: Ref<Record<string, any>>, item: Ref<Item>) {
		if (!currentVersion.value) return;
		saveVersionLoading.value = true;
		validationErrors.value = [];

		const payloadToValidate = mergeWith(
			{},
			defaultValues.value,
			item.value,
			edits.value,
			function (_from: any, to: any) {
				if (typeof to !== 'undefined') {
					return to;
				}
			},
		);

		const fields = pushGroupOptionsDown(fieldsWithPermissions.value);

		const errors = validateItem(payloadToValidate, fields, false, false, currentVersion.value);
		if (nestedValidationErrors.value?.length) errors.push(...nestedValidationErrors.value);

		if (errors.length > 0) {
			validationErrors.value = errors;
			saveVersionLoading.value = false;
			throw errors;
		}
Repository: directus/directus
File: app/src/interfaces/_system/system-display-template/system-display-template.vue (Vue)

Code before:
}>();

const { t } = useI18n();

const collectionsStore = useCollectionsStore();

const values = inject('values', ref<Record<string, any>>({}));

const collection = computed(() => {
	if (!props.collectionField) {
		if (props.collectionName) return props.collectionName;
		return null;
	}

	const collectionName = values.value[props.collectionField];

	const collectionExists = !!collectionsStore.collections.find(
		(collection) => collection.collection === collectionName,
	);

	if (collectionExists === false) return null;
	return collectionName;
});

const versioningEnabled = computed(() => values.value['versioning']);
const { versionField } = useVersionField(collection, props.injectVersionField ?? false, versioningEnabled.value);

const injectValue = computed(() => {
	if (!props.injectVersionField) return null;
	return versionField.value ? { fields: [versionField.value] } : null;
});

const { treeList, loadFieldRelations } = useFieldTree(collection, injectValue);

const tree = computed(() => {
	if (props.fields) {
		return { list: props.fields };
	}

	if (collection.value === null) {
		return null;
	}

	return { list: treeList.value, pathLoader: loadFieldRelations };
});
</script>

<template>
	<div class="system-display-template">
		<v-notice v-if="tree === null">
			{{ t('interfaces.system-display-template.select_a_collection') }}

Review comment (suggestion):
These two parameters must remain reactive. Suggested code below is untested!

```suggestion
const { versionField } = useVersionField(collection, toRef(props, 'injectVersionField'), versioningEnabled);
```

Code after:
}>();

const { t } = useI18n();

const collectionsStore = useCollectionsStore();

const values = inject('values', ref<Record<string, any>>({}));

const collection = computed(() => {
	if (!props.collectionField) {
		if (props.collectionName) return props.collectionName;
		return null;
	}

	const collectionName = values.value[props.collectionField];

	const collectionExists = !!collectionsStore.collections.find(
		(collection) => collection.collection === collectionName,
	);

	if (collectionExists === false) return null;
	return collectionName;
});

const versioningEnabled = computed(() => Boolean(values.value.versioning && props.injectVersionField));
const { fakeVersionField } = useFakeVersionField(collection, versioningEnabled);

const injectFields = computed(() => {
	return fakeVersionField.value ? { fields: [fakeVersionField.value] } : null;
});

const { treeList, loadFieldRelations } = useFieldTree(collection, injectFields);

const tree = computed(() => {
	if (props.fields) {
		return { list: props.fields };
	}

	if (collection.value === null) {
		return null;
	}

	return { list: treeList.value, pathLoader: loadFieldRelations };
});
</script>

<template>
	<div class="system-display-template">
		<v-notice v-if="tree === null">
			{{ t('interfaces.system-display-template.select_a_collection') }}
		</v-notice>
Repository: directus/directus
File: app/src/composables/use-fake-version-field.ts (TypeScript)

Code before:
import type { Field } from '@directus/types';
import { computed, unref, type Ref, type ComputedRef } from 'vue';
import { useI18n } from 'vue-i18n';

export function useFakeVersionField(
	collection: Ref<string | null>,
	injectVersionField: Ref<boolean>,
	versioningEnabled: Ref<boolean>,
): { fakeVersionField: ComputedRef<Field | null> } {
	const { t } = useI18n();

	const fakeVersionField = computed<Field | null>(() => {
		const collectionValue = unref(collection);
		if (!injectVersionField || !versioningEnabled || !collectionValue) return null;

		return {
			collection: collectionValue,
			field: '$version',
			schema: null,
			name: t('version'),
			type: 'integer',
			meta: {
				id: -1,
				collection: collectionValue,
				field: '$version',
				sort: null,
				special: null,
				interface: null,
				options: null,
				display: null,
				display_options: null,
				hidden: false,
				translations: null,
				readonly: true,
				width: 'full',
				group: null,
				note: null,
				required: false,
				conditions: null,

Review comment (suggestion):

```suggestion
		if (!injectVersionField.value || !versioningEnabled.value || !collectionValue) return null;
```

Code after:
import type { Field } from '@directus/types';
import { computed, ref, type Ref, type ComputedRef } from 'vue';
import { useI18n } from 'vue-i18n';
import { useVersions } from './use-versions';

type Choice = { text: string; value: string | null };

export function useFakeVersionField(
	collection: Ref<string | null>,
	versioningEnabled: Ref<boolean>,
	includeChoices = ref(false),
): { fakeVersionField: ComputedRef<Field | null> } {
	const { t } = useI18n();

	// If `includeChoices` is `true`, the versions will be fetched, because they are only fetched if the `isSingleton` parameter of `useVersions` is `true`.
	const { versions } = useVersions(collection as Ref<string>, includeChoices, ref(null));

	const fakeVersionField = computed<Field | null>(() => {
		if (!versioningEnabled.value || !collection.value) return null;

		const choices = getChoices();

		return {
			collection: collection.value,
			field: '$version',
			schema: null,
			name: t('version'),
			type: 'string',
			meta: {
				field: '$version',
				collection: collection.value,
				id: -1,
				conditions: null,
				display: null,
				display_options: null,
				group: null,
				hidden: false,
				interface: 'select-dropdown',
				note: null,
Repository: directus/directus
File: app/src/interfaces/_system/system-filter/input-group.vue (Vue)

Code before:
<script setup lang="ts">
import { useFakeVersionField } from '@/composables/use-fake-version-field';
import { useFieldsStore } from '@/stores/fields';
import { useRelationsStore } from '@/stores/relations';
import { FieldFilter } from '@directus/types';
import { clone, get } from 'lodash';
import { computed, ref } from 'vue';
import { useI18n } from 'vue-i18n';
import InputComponent from './input-component.vue';
import { fieldToFilter, getComparator, getField } from './utils';
import { useCollection } from '@directus/composables';

const props = defineProps<{
	field: FieldFilter;
	collection: string;
}>();

const emit = defineEmits<{
	(e: 'update:field', value: FieldFilter): void;
}>();

const fieldsStore = useFieldsStore();
const relationsStore = useRelationsStore();
const { t } = useI18n();

const { info: collectionInfo } = useCollection(computed(() => props.collection));

const versioningEnabled = computed(() => collectionInfo.value?.meta?.versioning ?? false);

const { fakeVersionField } = useFakeVersionField(
	computed(() => props.collection),
	ref(true),

Review comment (suggestion):
```suggestion
import { computed, ref, toRef } from 'vue';
```

Code after:
<script setup lang="ts">
import { useFakeVersionField } from '@/composables/use-fake-version-field';
import { useFieldsStore } from '@/stores/fields';
import { useRelationsStore } from '@/stores/relations';
import { FieldFilter } from '@directus/types';
import { clone, get } from 'lodash';
import { computed, toRef } from 'vue';
import { useI18n } from 'vue-i18n';
import InputComponent from './input-component.vue';
import { fieldToFilter, getComparator, getField } from './utils';
import { useCollection } from '@directus/composables';

const props = defineProps<{
	field: FieldFilter;
	collection: string;
}>();

const emit = defineEmits<{
	(e: 'update:field', value: FieldFilter): void;
}>();

const fieldsStore = useFieldsStore();
const relationsStore = useRelationsStore();
const { t } = useI18n();

const field = computed(() => getField(props.field));
const isVersionField = computed(() => field.value === '$version');

const { info: collectionInfo } = useCollection(computed(() => props.collection));
const versioningEnabled = computed(() => !!collectionInfo.value?.meta?.versioning && isVersionField.value);
const { fakeVersionField } = useFakeVersionField(toRef(props, 'collection'), versioningEnabled, isVersionField);
Repository: directus/directus
File: app/src/modules/content/routes/item.vue (Vue)

Code before:
function discardAndLeave() {
	if (!leaveTo.value) return;
	edits.value = {};
	confirmLeave.value = false;
	router.push(leaveTo.value);
}

function discardAndStay() {
	edits.value = {};
	confirmLeave.value = false;
}

function revert(values: Record<string, any>) {
	edits.value = {
		...edits.value,
		...values,
	};
}
</script>

<template>
	<content-not-found
		v-if="error || !collectionInfo || (collectionInfo?.meta?.singleton === true && primaryKey !== null)"
	/>

	<private-view :data-collection="collectionInfo.collection" v-else v-model:split-view="splitView" :split-view-min-width="310" :title="title">
		<template v-if="collectionInfo.meta && collectionInfo.meta.singleton === true" #title>
			<h1 class="type-title">
				{{ collectionInfo.name }}
			</h1>
		</template>

		<template v-else-if="isNew === false && collectionInfo.meta && collectionInfo.meta.display_template" #title>
			<v-skeleton-loader v-if="loading" class="title-loader" type="text" />

			<h1 v-else class="type-title">
				<render-template
					:collection="collectionInfo.collection"
					:item="templateData"
					:template="collectionInfo.meta.display_template"
				/>
			</h1>
		</template>

		<template #title-outer:prepend>
			<v-button
				v-if="collectionInfo.meta && collectionInfo.meta.singleton === true"
				class="header-icon"
				rounded
				icon
				secondary

Review comment (suggestion):
minor formatting issue
```suggestion
	<private-view
		v-else
		v-model:split-view="splitView"
		:data-collection="collectionInfo.collection"
		:split-view-min-width="310"
		:title="title"
	>
```

Code after:

		if (isArchived.value === true) {
			router.push(getCollectionRoute(props.collection));
		} else {
			confirmArchive.value = false;
		}
	} catch {
		// `archive` will show the unexpected error dialog
	}
}

function discardAndLeave() {
	if (!leaveTo.value) return;
	edits.value = {};
	confirmLeave.value = false;
	router.push(leaveTo.value);
}

function discardAndStay() {
	edits.value = {};
	confirmLeave.value = false;
}

function revert(values: Record<string, any>) {
	edits.value = {
		...edits.value,
		...values,
	};
}

const shouldShowVersioning = computed(
	() =>
		collectionInfo.value?.meta?.versioning &&
		!isNew.value &&
		internalPrimaryKey.value !== '+' &&
		readVersionsAllowed.value &&
		!versionsLoading.value,
);
</script>

<template>
	<content-not-found
		v-if="error || !collectionInfo || (collectionInfo?.meta?.singleton === true && primaryKey !== null)"
	/>

	<private-view
		v-else
		v-model:split-view="splitView"
		:class="{ 'has-content-versioning': shouldShowVersioning }"
		:split-view-min-width="310"
		:title="title"
Repository: directus/directus
File: api/src/services/tus/server.ts (TypeScript)

Code before:
				const newFile = await service.readOne(file.tus_data['metadata']['replace_id']);
				const updateFields = pick(file, ['filename_download', 'filesize', 'type']);

				const metadata = await extractMetadata(newFile.storage, {
					...newFile,
					...updateFields,
				});

				await service.updateOne(file.tus_data['metadata']['replace_id'], {
					...updateFields,
					...metadata,
				});

				await service.deleteOne(file.id);
			} else {
				const metadata = await extractMetadata(file.storage, file);

				await service.updateOne(file.id, {
					...metadata,
					tus_id: null,
					tus_data: null,
				});
			}

			const eventMeta = {
				payload: await service.readOne(file.id),
				key: file.id,
				collection: 'directus_files',
			};

			const eventContext = {
				database: getDatabase(),
				schema: req.schema,
				accountability: req.accountability,
			};

			emitter.emitAction('files.upload', eventMeta, eventContext);

			return res;
		},
		generateUrl(_req, opts) {
			return env['PUBLIC_URL'] + '/files/tus/' + opts.id;
		},
		relativeLocation: String(env['PUBLIC_URL']).startsWith('http'),
	});

	server.on(EVENTS.POST_CREATE, async (_req, res, upload) => {
		res.setHeader('Directus-File-Id', upload.metadata!['id']!);
	});

	return [server, cleanup];

Review comment (bug):
This will fail on `replace` as we delete the file after it is replaced, see L84. 

The following I believe should account for that
```suggestion
				payload: await service.readOne(file.tus_data?.['metadata']?.['replace_id'] ?? file.id),
```

Code after:
			// update metadata when file is replaced
			if (file.tus_data?.['metadata']?.['replace_id']) {
				const newFile = await service.readOne(file.tus_data['metadata']['replace_id']);
				const updateFields = pick(file, ['filename_download', 'filesize', 'type']);

				const metadata = await extractMetadata(newFile.storage, {
					...newFile,
					...updateFields,
				});

				await service.updateOne(file.tus_data['metadata']['replace_id'], {
					...updateFields,
					...metadata,
				});

				fileData = {
					...newFile,
					...updateFields,
					...metadata,
					id: file.tus_data['metadata']['replace_id'],
				};

				await service.deleteOne(file.id);
			} else {
				const metadata = await extractMetadata(file.storage, file);

				await service.updateOne(file.id, {
					...metadata,
					tus_id: null,
					tus_data: null,
				});

				fileData = {
					...file,
					...metadata,
					tus_id: null,
					tus_data: null,
				};
			}

			emitter.emitAction(
				'files.upload',
				{
					payload: fileData,
					key: fileData.id,
					collection: 'directus_files',
				},
				{
					database: getDatabase(),
					schema: req.schema,
					accountability: req.accountability,