ronantakizawa/codereview-bench huggingface dataset:
Repository: directus/directus
File: app/src/modules/settings/routes/flows/components/logs-drawer.vue (Vue)

Code before:
<script setup lang="ts">
import { useExtensions } from '@/extensions';
import { computed, toRefs, unref } from 'vue';
import { useI18n } from 'vue-i18n';
import { getTriggers } from '../triggers';
import { FlowRaw } from '@directus/types';
import VDetail from '@/components/v-detail.vue';

const props = defineProps<{
	flow: FlowRaw;
	revision: any;
	closeDrawer: () => void;
}>();

const { flow, revision, closeDrawer } = toRefs(props);

const { t } = useI18n();

const { triggers } = getTriggers();
const { operations } = useExtensions();

const usedTrigger = computed(() => {
	return triggers.find((trigger) => trigger.id === unref(flow).trigger);
});

const triggerData = computed(() => {
	if (!unref(revision)?.data) return { trigger: null, accountability: null, options: null };

	const { data } = unref(revision)?.data as any;

	return {
		trigger: data.$trigger,
		accountability: data.$accountability,
		options: props.flow.options,
	};
});


Review comment (suggestion):
Let’s stick with the `emit` approach

```suggestion
```

Code after:
<script setup lang="ts">
import { useExtensions } from '@/extensions';
import { computed, toRefs, unref } from 'vue';
import { useI18n } from 'vue-i18n';
import { getTriggers } from '../triggers';
import { FlowRaw } from '@directus/types';
import VDetail from '@/components/v-detail.vue';
import { Revision } from '@/types/revisions';

const props = defineProps<{
	flow: FlowRaw;
	revision: Revision;
}>();

const { flow, revision } = toRefs(props);

const { t } = useI18n();

const { triggers } = getTriggers();
const { operations } = useExtensions();

const usedTrigger = computed(() => {
	return triggers.find((trigger) => trigger.id === unref(flow).trigger);
});

const triggerData = computed(() => {
	if (!unref(revision)?.data) return { trigger: null, accountability: null, options: null };

	const { data } = unref(revision)?.data as any;

	return {
		trigger: data.$trigger,
		accountability: data.$accountability,
		options: flow.value.options,
	};
});
Repository: directus/directus
File: app/src/modules/settings/routes/flows/components/logs-sidebar-detail.vue (Vue)

Code before:
			action: Action.RUN,
			full: true,
		},
	);

watch(
	() => page.value,
	(newPage) => {
		refresh(newPage);
	},
);

watch(showFailedOnly, (showFailed) => {
	if (!revisionsByDate.value) return;

	if (showFailed) {
		revisionsByDate.value = revisionsByDate.value
			.map((group) => ({
				...group,
				revisions: group.revisions.filter((r) => r.status === 'reject'),
			}))
			.filter((group) => group.revisions.length > 0);
	} else {
		refresh(page.value);
	}
});

onMounted(() => {
	getRevisionsCount();
	if (open.value) getRevisions();
});

function onToggle(open: boolean) {
	if (open && revisionsByDate.value === null) getRevisions();
}
</script>

<template>
	<sidebar-detail
		:title
		icon="fact_check"
		:badge="!loadingCount && revisionsCount > 0 ? abbreviateNumber(revisionsCount) : null"
		@toggle="onToggle"
	>
		<v-progress-linear v-if="!revisionsByDate && loading" indeterminate />

		<div v-else-if="revisionsCount === 0" class="empty">{{ t('no_logs') }}</div>

		<template v-else>
			<button class="toggle-failed" :class="{ active: showFailedOnly }" @click="showFailedOnly = !showFailedOnly">
				<v-icon v-if="!showFailedOnly" name="circle" small />

Review comment (suggestion):
There are some issues with the pagination — see the video below.

https://github.com/user-attachments/assets/8a18e134-3ee3-4493-ae99-04034a2948c9

Code after:
			action: Action.RUN,
			full: true,
		},
	);

watch([() => page.value, () => showFailedOnly.value], async () => {
	await refresh(page.value);
	if (showFailedOnly.value) filterRevisions();
});

function filterRevisions() {
	if (!revisionsByDate.value) return;

	revisionsByDate.value = revisionsByDate.value
		.map((group) => ({
			...group,
			revisions: group.revisions.filter((r) => r.status === 'reject'),
		}))
		.filter((group) => group.revisions.length > 0);
}

onMounted(() => {
	getRevisionsCount();
	if (open.value) getRevisions();
});

function onToggle(open: boolean) {
	if (open && revisionsByDate.value === null) getRevisions();
}
</script>

<template>
	<sidebar-detail
		:title
		icon="fact_check"
		:badge="!loadingCount && revisionsCount > 0 ? abbreviateNumber(revisionsCount) : null"
		@toggle="onToggle"
	>
		<v-progress-linear v-if="!revisionsByDate && loading" indeterminate />

		<div v-else-if="revisionsCount === 0" class="empty">{{ t('no_logs') }}</div>

		<template v-else>
			<button class="toggle-failed" :class="{ active: showFailedOnly }" @click="showFailedOnly = !showFailedOnly">
				<v-icon v-if="!showFailedOnly" name="circle" small />
				<v-icon v-else name="cancel" small />
				{{ t('show_failed_only') }}
			</button>

			<div v-if="!revisionsByDate?.length" class="empty">{{ t('no_logs_on_page') }}</div>
Repository: directus/directus
File: sdk/src/auth/types.ts (TypeScript)

Code before:
export type AuthenticationMode = 'json' | 'cookie' | 'session';

export type LoginPayload = { email: string; password: string } | { identifier: string; password: string };

export type LoginOptions = {
	/** The user's one-time-password (if MFA is enabled). */
	otp?: string;
	/** Whether to retrieve the refresh token in the JSON response, or in a httpOnly cookie. One of `json`, `cookie` or `session`. Defaults to `cookie`. */
	mode?: AuthenticationMode;
	/** Use a specific authentication provider (does not work for SSO that relies on browser redirects). */
	provider?: string;
};

export type LogoutOptions = {
	refresh_token?: string;
	mode?: AuthenticationMode;
};

export type RefreshOptions = {
	refresh_token?: string;
	mode?: AuthenticationMode;
};

export interface AuthenticationData {
	access_token: string | null;
	refresh_token: string | null;
	expires: number | null;
	expires_at: number | null;

Review comment (suggestion):
May be worth improving this type def as it now allows all 3 to be passed instead of email OR identifier.

Code after:
export type AuthenticationMode = 'json' | 'cookie' | 'session';

export type LocalLoginPayload = { email: string; password: string };
export type LDAPLoginPayload = { identifier: string; password: string };
export type LoginPayload = LocalLoginPayload | LDAPLoginPayload;

export type LoginOptions = {
	/** The user's one-time-password (if MFA is enabled). */
	otp?: string;
	/** Whether to retrieve the refresh token in the JSON response, or in a httpOnly cookie. One of `json`, `cookie` or `session`. Defaults to `cookie`. */
	mode?: AuthenticationMode;
	/** Use a specific authentication provider (does not work for SSO that relies on browser redirects). */
	provider?: string;
};

export type LogoutOptions = {
	refresh_token?: string;
	mode?: AuthenticationMode;
};

export type RefreshOptions = {
	refresh_token?: string;
	mode?: AuthenticationMode;
};

export interface AuthenticationData {
	access_token: string | null;
	refresh_token: string | null;
Repository: directus/directus
File: app/src/interfaces/select-color/select-color.vue (Vue)

Code before:
					// Color or cssVar could not resolve the color to a color in JS, however, the CSS Var may still be a valid color.
					// So we keep the input value as is and set the internal color to null.
					// This way the user can still edit the input and we can still show the color in the swatch.
					// The color editor (rgb/hsl) will show the color as black (0,0,0) in this case.
					color.value = null;
				}
			}
		},
	});

	return { rgb, hsl, hex, alpha, color, input, onChanged };

	function onChanged() {
		if (!input.value) {
			unsetColor();
			return;
		}

		if (isCssVarUtil(input.value)) return;

		try {
			// If the input is a valid color, we set the color and emit the input as a hex value which is consistent with the dropdown selector and HTML color picker
			const newColor = Color(input.value);
			setColor(newColor);
		} catch {
			// The input is not a valid color, but we still want to let the user edit/type in the input so we emit the input
			unsetColor();
		}
	}

	function setColor(newColor: ColorInstance | null) {
		color.value = newColor;

		if (newColor === null) {
			unsetColor();
		} else {
			emit('input', getHexa());
		}
	}

	function roundColorValues(arr: number[]): number[] {
		if (arr.length === 4) {
			// Do not round the opacity
			return [...arr.slice(0, -1).map((x) => Math.round(x)), arr[3]!];
		}

		return arr.map((x) => Math.round(x));
	}
}
</script>


Review comment (suggestion):
```suggestion
			// The input is not a valid color, but we still want to let the user edit/type in the input so we emit null to prevent using an invalid value
```

Code after:
					// Color or cssVar could not resolve the color to a color in JS, however, the CSS Var may still be a valid color.
					// So we keep the input value as is and set the internal color to null.
					// This way the user can still edit the input and we can still show the color in the swatch.
					// The color editor (rgb/hsl) will show the color as black (0,0,0) in this case.
					color.value = null;
				}
			}
		},
	});

	return { rgb, hsl, hex, alpha, color, input, onChanged };

	function onChanged() {
		if (!input.value) {
			unsetColor();
			return;
		}

		if (isCssVarUtil(input.value)) return;

		try {
			// If the input is a valid color, we set the color and emit the input as a hex value which is consistent with the dropdown selector and HTML color picker
			const newColor = Color(input.value);
			setColor(newColor);
		} catch {
			// The input is not a valid color, but we still want to let the user edit/type in the input so we emit null to prevent using an invalid value
			unsetColor();
		}
	}

	function setColor(newColor: ColorInstance | null) {
		color.value = newColor;

		if (newColor === null) {
			unsetColor();
		} else {
			emit('input', getHexa());
		}
	}

	function roundColorValues(arr: number[]): number[] {
		if (arr.length === 4) {
			// Do not round the opacity
			return [...arr.slice(0, -1).map((x) => Math.round(x)), arr[3]!];
		}

		return arr.map((x) => Math.round(x));
	}
}
</script>
Repository: directus/directus
File: app/src/modules/files/routes/item.vue (Vue)

Code before:
						<v-button kind="danger" :loading="deleting" @click="deleteAndQuit">
							{{ t('delete_label') }}
						</v-button>
					</v-card-actions>
				</v-card>
			</v-dialog>

			<v-dialog v-if="isNew === false" v-model="moveToDialogActive" @esc="moveToDialogActive = false">
				<template #activator="{ on }">
					<v-button v-tooltip.bottom="t('move_to_folder')" rounded icon :disabled="item === null" secondary @click="on">
						<v-icon name="folder_move" />
					</v-button>
				</template>

				<v-card>
					<v-card-title>{{ t('move_to_folder') }}</v-card-title>

					<v-card-text>
						<folder-picker v-model="selectedFolder" />
					</v-card-text>

					<v-card-actions>
						<v-button secondary @click="moveToDialogActive = false">
							{{ t('cancel') }}
						</v-button>
						<v-button :loading="moving" :disabled="!updateAllowed" @click="moveToFolder">
							{{ t('move') }}
						</v-button>
					</v-card-actions>
				</v-card>
			</v-dialog>
			<v-button
				v-tooltip.bottom="t('download')"
				secondary
				icon
				rounded
				:download="item?.filename_download"
				:href="getAssetUrl(props.primaryKey, true)"
			>
				<v-icon name="download" />
			</v-button>

			<v-button
				v-if="item?.type?.includes('image') && updateAllowed"
				v-tooltip.bottom="updateAllowed ? t('edit') : t('not_allowed')"
				rounded
				icon
				secondary
				@click="editActive = true"
			>
				<v-icon name="tune" />

Review comment (suggestion):
Instead of adding the condition to the button in the modal, add it to the button in the `activator` slot above, to ensure we are not able to open the modal. Also add the `'not_allowed'` label.

Code after:
						<v-button kind="danger" :loading="deleting" @click="deleteAndQuit">
							{{ t('delete_label') }}
						</v-button>
					</v-card-actions>
				</v-card>
			</v-dialog>

			<v-dialog v-if="isNew === false" v-model="moveToDialogActive" @esc="moveToDialogActive = false">
				<template #activator="{ on }">
					<v-button
						v-tooltip.bottom="item === null || !updateAllowed ? t('not_allowed') : t('move_to_folder')"
						rounded
						icon
						secondary
						:disabled="item === null || !updateAllowed"
						@click="on"
					>
						<v-icon name="folder_move" />
					</v-button>
				</template>

				<v-card>
					<v-card-title>{{ t('move_to_folder') }}</v-card-title>

					<v-card-text>
						<folder-picker v-model="selectedFolder" />
					</v-card-text>

					<v-card-actions>
						<v-button secondary @click="moveToDialogActive = false">
							{{ t('cancel') }}
						</v-button>
						<v-button :loading="moving" @click="moveToFolder">
							{{ t('move') }}
						</v-button>
					</v-card-actions>
				</v-card>
			</v-dialog>
			<v-button
				v-tooltip.bottom="t('download')"
				secondary
				icon
				rounded
				:download="item?.filename_download"
				:href="getAssetUrl(props.primaryKey, true)"
			>
				<v-icon name="download" />
			</v-button>

			<v-button
				v-if="item?.type?.includes('image') && updateAllowed"
Repository: directus/directus
File: app/src/modules/files/routes/collection.vue (Vue)

Code before:
					<template #activator="{ on }">
						<v-button
							v-tooltip.bottom="batchEditAllowed ? t('move_to_folder') : t('not_allowed')"
							rounded
							icon
							class="folder"
							secondary
							:disabled="!batchEditAllowed"
							@click="on"
						>
							<v-icon name="folder_move" />
						</v-button>
					</template>

					<v-card>
						<v-card-title>{{ t('move_to_folder') }}</v-card-title>

						<v-card-text>
							<folder-picker v-model="selectedFolder" />
						</v-card-text>

						<v-card-actions>
							<v-button secondary @click="moveToDialogActive = false">
								{{ t('cancel') }}
							</v-button>
							<v-button :loading="moving" :disabled="!batchEditAllowed" @click="moveToFolder">
								{{ t('move') }}
							</v-button>
						</v-card-actions>
					</v-card>
				</v-dialog>

				<v-dialog v-if="selection.length > 0" v-model="confirmDelete" @esc="confirmDelete = false">
					<template #activator="{ on }">
						<v-button
							v-tooltip.bottom="batchDeleteAllowed ? t('delete_label') : t('not_allowed')"
							:disabled="batchDeleteAllowed !== true"
							rounded
							icon
							class="action-delete"
							secondary
							@click="on"
						>
							<v-icon name="delete" outline />
						</v-button>
					</template>

					<v-card>
						<v-card-title>{{ t('batch_delete_confirm', selection.length) }}</v-card-title>

						<v-card-actions>

Review comment (suggestion):
Let’s remove the redundancy here, as well as in the item.vue component.

```suggestion
							<v-button :loading="moving" @click="moveToFolder">
```

Code after:
					<template #activator="{ on }">
						<v-button
							v-tooltip.bottom="batchEditAllowed ? t('move_to_folder') : t('not_allowed')"
							rounded
							icon
							class="folder"
							secondary
							:disabled="!batchEditAllowed"
							@click="on"
						>
							<v-icon name="folder_move" />
						</v-button>
					</template>

					<v-card>
						<v-card-title>{{ t('move_to_folder') }}</v-card-title>

						<v-card-text>
							<folder-picker v-model="selectedFolder" />
						</v-card-text>

						<v-card-actions>
							<v-button secondary @click="moveToDialogActive = false">
								{{ t('cancel') }}
							</v-button>
							<v-button :loading="moving" @click="moveToFolder">
								{{ t('move') }}
							</v-button>
						</v-card-actions>
					</v-card>
				</v-dialog>

				<v-dialog v-if="selection.length > 0" v-model="confirmDelete" @esc="confirmDelete = false">
					<template #activator="{ on }">
						<v-button
							v-tooltip.bottom="batchDeleteAllowed ? t('delete_label') : t('not_allowed')"
							:disabled="batchDeleteAllowed !== true"
							rounded
							icon
							class="action-delete"
							secondary
							@click="on"
						>
							<v-icon name="delete" outline />
						</v-button>
					</template>

					<v-card>
						<v-card-title>{{ t('batch_delete_confirm', selection.length) }}</v-card-title>

						<v-card-actions>
Repository: directus/directus
File: api/src/services/relations.ts (TypeScript)

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

		const metaReadQuery: Query = {
			limit: -1,
		};

		if (collection) {
			metaReadQuery.filter = {
				many_collection: {
					_eq: collection,
				},
			};
		}

		const metaRows = [
			...(await this.relationsItemService.readByQuery(metaReadQuery, opts)),
			...systemRelationRows,
		].filter((metaRow) => {
			if (!collection) return true;
			return metaRow.many_collection === collection;
		});

		const schemaRows = bypassCache ? await this.schemaInspector.foreignKeys() : await this.foreignKeys(collection);
		const results = this.stitchRelations(metaRows, schemaRows);
		return await this.filterForbidden(results);
	}

	async readOne(collection: string, field: string): Promise<Relation> {
		if (this.accountability && this.accountability.admin !== true) {
			await validateAccess(
				{
					accountability: this.accountability,
					action: 'read',
					collection: 'directus_relations',
				},
				{
					schema: this.schema,
					knex: this.knex,
				},
			);

			const allowedFields = await fetchAllowedFields(
				{ collection, action: 'read', accountability: this.accountability },
				{ schema: this.schema, knex: this.knex },
			);

			if (allowedFields.includes('*') === false && allowedFields.includes(field) === false) {
				throw new ForbiddenError();

Review comment (performance):

```suggestion
		const schemaRows = bypassCache ? await this.schemaInspector.foreignKeys().filter((row) => row.table === collection) : await this.foreignKeys(collection);
```

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

		const metaReadQuery: Query = {
			limit: -1,
		};

		if (collection) {
			metaReadQuery.filter = {
				many_collection: {
					_eq: collection,
				},
			};
		}

		const metaRows = [
			...(await this.relationsItemService.readByQuery(metaReadQuery, opts)),
			...systemRelationRows,
		].filter((metaRow) => {
			if (!collection) return true;
			return metaRow.many_collection === collection;
		});

		let schemaRows = bypassCache ? await this.schemaInspector.foreignKeys() : await this.foreignKeys(collection);

		if (collection && bypassCache) {
			schemaRows = schemaRows.filter((row) => row.table === collection);
		}

		const results = this.stitchRelations(metaRows, schemaRows);
		return await this.filterForbidden(results);
	}

	async readOne(collection: string, field: string): Promise<Relation> {
		if (this.accountability && this.accountability.admin !== true) {
			await validateAccess(
				{
					accountability: this.accountability,
					action: 'read',
					collection: 'directus_relations',
				},
				{
					schema: this.schema,
					knex: this.knex,
				},
			);

			const allowedFields = await fetchAllowedFields(
				{ collection, action: 'read', accountability: this.accountability },
Repository: directus/directus
File: app/src/interfaces/file/file.vue (Vue)

Code before:

const { url, isValidURL, loading: urlLoading, importFromURL } = useURLImport();

const editDrawerActive = ref(false);

const edits = computed(() => {
	if (!props.value || typeof props.value !== 'object') return {};

	return props.value;
});

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

const customFilter = computed(() => {
	return parseFilter(
		deepMap(props.filter, (val: any) => {
			if (val && typeof val === 'string') {
				return render(val, values.value);
			}

			return val;
		}),
	);
});

const _disabled = computed(() => {
	return props.disabled || (props.enableCreate === false && props.enableSelect === false);
});

function setSelection(selection: (string | number)[] | null) {
	if (selection![0]) {
		update(selection![0]);
	} else {
		remove();
	}
}

function onUpload(fileInfo: FileInfo) {
	file.value = fileInfo;
	activeDialog.value = null;
	update(fileInfo.id);
}

function useURLImport() {
	const url = ref('');
	const loading = ref(false);

	const isValidURL = computed(() => {
		try {
			new URL(url.value);
			return true;

Review comment (suggestion):
```suggestion
const disabled = computed(() => {
```

Lets rename `_disabled` to `disabled`. Variable names starting with underscore imply that it is unused in typescript.

Code after:

const { url, isValidURL, loading: urlLoading, importFromURL } = useURLImport();

const editDrawerActive = ref(false);

const edits = computed(() => {
	if (!props.value || typeof props.value !== 'object') return {};

	return props.value;
});

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

const customFilter = computed(() => {
	return parseFilter(
		deepMap(props.filter, (val: unknown) => {
			if (val && typeof val === 'string') {
				return render(val, values.value);
			}

			return val;
		}),
	);
});

const internalDisabled = computed(() => {
	return props.disabled || (props.enableCreate === false && props.enableSelect === false);
});

function setSelection(selection: (string | number)[] | null) {
	if (selection![0]) {
		update(selection![0]);
	} else {
		remove();
	}
}

function onUpload(fileInfo: FileInfo) {
	file.value = fileInfo;
	activeDialog.value = null;
	update(fileInfo.id);
}

function useURLImport() {
	const url = ref('');
	const loading = ref(false);

	const isValidURL = computed(() => {
		try {
			new URL(url.value);
			return true;