ronantakizawa/codereview-bench huggingface dataset:
Repository: directus/directus
File: app/src/modules/content/components/version-menu.vue (Vue)

Code before:
}

function useCreateDialog() {
	const createDialogActive = ref(false);
	const creating = ref(false);
	const newVersionKey = ref<string | null>(null);
	const newVersionName = ref<string | null>(null);

	watch(
		newVersionName,
		(newName, oldName) => {
			if (
				newName === null ||
				newVersionKey.value ===
					slugify(oldName ?? '', {
						separator: '_',
					})
			) {
				newVersionKey.value = slugify(newName ?? '', {
					separator: '_',
				});
			}
		},
		{ immediate: true },
	);

	return {
		newVersionKey,
		newVersionName,
		createDialogActive,
		creating,
		createVersion,
		closeCreateDialog,
	};

	async function createVersion() {
		if (!unref(primaryKey) || unref(primaryKey) === '+') return;

		creating.value = true;

		try {
			const {
				data: { data: version },
			} = await api.post(`/versions`, {
				key: unref(newVersionKey),
				...(unref(newVersionName) ? { name: unref(newVersionName) } : {}),
				collection: unref(collection),
				item: String(unref(primaryKey)),
			});

			emit('add', version);

Review comment (suggestion):
```suggestion
	watch(
		newVersionName,
		(newName, oldName) => {
			if (
				newName === null ||
				newVersionKey.value ===
					slugify(oldName ?? '', {
						separator: '-',
					})
			) {
				newVersionKey.value = slugify(newName ?? '', {
					separator: '-',
				});
			}
		},
		{ immediate: true },
	);

```
In the current app when typing into the key field and pressing "space" it inserts a `-` rather than a `_`

Code after:
}

function useCreateDialog() {
	const createDialogActive = ref(false);
	const creating = ref(false);
	const newVersionKey = ref<string | null>(null);
	const newVersionName = ref<string | null>(null);

	watch(
		newVersionName,
		(newName, oldName) => {
			if (
				newName === null ||
				newVersionKey.value ===
					slugify(oldName ?? '', {
						separator: '-',
					})
			) {
				newVersionKey.value = slugify(newName ?? '', {
					separator: '-',
				});
			}
		},
		{ immediate: true },
	);

	return {
		newVersionKey,
		newVersionName,
		createDialogActive,
		creating,
		createVersion,
		closeCreateDialog,
	};

	async function createVersion() {
		if (!unref(primaryKey) || unref(primaryKey) === '+') return;

		creating.value = true;

		try {
			const {
				data: { data: version },
			} = await api.post(`/versions`, {
				key: unref(newVersionKey),
				...(unref(newVersionName) ? { name: unref(newVersionName) } : {}),
				collection: unref(collection),
				item: String(unref(primaryKey)),
			});

			emit('add', version);
Repository: directus/directus
File: api/src/permissions/utils/extract-required-dynamic-variable-context.ts (TypeScript)

Code before:
import type { Permission } from '@directus/types';
import { deepMap } from '@directus/utils';

export interface DynamicVariableContext {
	$CURRENT_USER: Set<string>;
	$CURRENT_ROLE: Set<string>;
	$CURRENT_ROLES: Set<string>;
	$CURRENT_POLICIES: Set<string>;
}

export function extractRequiredDynamicVariableContextForPermissions(permissions: Permission[]) {
	let permissionContext: DynamicVariableContext = {
		$CURRENT_USER: new Set<string>(),
		$CURRENT_ROLE: new Set<string>(),
		$CURRENT_ROLES: new Set<string>(),
		$CURRENT_POLICIES: new Set<string>(),
	};

	for (const permission of permissions) {
		permissionContext = merge_contexts(
			permissionContext,
			extractRequiredDynamicVariableContext(permission.permissions),
		);

		permissionContext = merge_contexts(permissionContext, extractRequiredDynamicVariableContext(permission.validation));
		permissionContext = merge_contexts(permissionContext, extractRequiredDynamicVariableContext(permission.presets));
	}

	return permissionContext;
}

export function extractRequiredDynamicVariableContext(val: any) {
	const permissionContext: DynamicVariableContext = {
		$CURRENT_USER: new Set<string>(),
		$CURRENT_ROLE: new Set<string>(),
		$CURRENT_ROLES: new Set<string>(),

Review comment (security):
```suggestion
export function extractRequiredDynamicVariablePermissionContext(permissions: Permission[]) {
```
The above may be more appropriate?

Code after:
import type { Permission } from '@directus/types';
import { deepMap } from '@directus/utils';

export interface DynamicVariableContext {
	$CURRENT_USER: Set<string>;
	$CURRENT_ROLE: Set<string>;
	$CURRENT_ROLES: Set<string>;
	$CURRENT_POLICIES: Set<string>;
}

export function extractRequiredDynamicVariableContextForPermissions(permissions: Permission[]) {
	let permissionContext: DynamicVariableContext = {
		$CURRENT_USER: new Set<string>(),
		$CURRENT_ROLE: new Set<string>(),
		$CURRENT_ROLES: new Set<string>(),
		$CURRENT_POLICIES: new Set<string>(),
	};

	for (const permission of permissions) {
		permissionContext = mergeContexts(permissionContext, extractRequiredDynamicVariableContext(permission.permissions));

		permissionContext = mergeContexts(permissionContext, extractRequiredDynamicVariableContext(permission.validation));
		permissionContext = mergeContexts(permissionContext, extractRequiredDynamicVariableContext(permission.presets));
	}

	return permissionContext;
}

export function extractRequiredDynamicVariableContext(val: any) {
	const permissionContext: DynamicVariableContext = {
		$CURRENT_USER: new Set<string>(),
		$CURRENT_ROLE: new Set<string>(),
		$CURRENT_ROLES: new Set<string>(),
		$CURRENT_POLICIES: new Set<string>(),
	};
Repository: directus/directus
File: api/src/permissions/utils/extract-required-dynamic-variable-context.ts (TypeScript)

Code before:
	const permissionContext: DynamicVariableContext = {
		$CURRENT_USER: new Set<string>(),
		$CURRENT_ROLE: new Set<string>(),
		$CURRENT_ROLES: new Set<string>(),
		$CURRENT_POLICIES: new Set<string>(),
	};

	deepMap(val, extractPermissionData);

	return permissionContext;

	function extractPermissionData(val: any) {
		for (const placeholder of [
			'$CURRENT_USER',
			'$CURRENT_ROLE',
			'$CURRENT_ROLES',
			'$CURRENT_POLICIES',
		] as (keyof typeof permissionContext)[]) {
			if (typeof val === 'string' && val.startsWith(`${placeholder}.`)) {
				permissionContext[placeholder].add(val.replace(`${placeholder}.`, ''));
			}
		}
	}
}

function merge_contexts(context1: DynamicVariableContext, context2: DynamicVariableContext) {
	const permissionContext: DynamicVariableContext = {
		$CURRENT_USER: new Set([...context1.$CURRENT_USER, ...context2.$CURRENT_USER]),
		$CURRENT_ROLE: new Set([...context1.$CURRENT_ROLE, ...context2.$CURRENT_ROLE]),
		$CURRENT_ROLES: new Set([...context1.$CURRENT_ROLES, ...context2.$CURRENT_ROLES]),
		$CURRENT_POLICIES: new Set([...context1.$CURRENT_POLICIES, ...context2.$CURRENT_POLICIES]),
	};

	return permissionContext;
}


Review comment (suggestion):
To be inline with js naming conventions this should be
```suggestion
function mergeContexts(context1: DynamicVariableContext, context2: DynamicVariableContext) {
```

Code after:
		$CURRENT_ROLES: new Set<string>(),
		$CURRENT_POLICIES: new Set<string>(),
	};

	deepMap(val, extractPermissionData);

	return permissionContext;

	function extractPermissionData(val: any) {
		for (const placeholder of [
			'$CURRENT_USER',
			'$CURRENT_ROLE',
			'$CURRENT_ROLES',
			'$CURRENT_POLICIES',
		] as (keyof typeof permissionContext)[]) {
			if (typeof val === 'string' && val.startsWith(`${placeholder}.`)) {
				permissionContext[placeholder].add(val.replace(`${placeholder}.`, ''));
			}
		}
	}
}

function mergeContexts(context1: DynamicVariableContext, context2: DynamicVariableContext) {
	const permissionContext: DynamicVariableContext = {
		$CURRENT_USER: new Set([...context1.$CURRENT_USER, ...context2.$CURRENT_USER]),
		$CURRENT_ROLE: new Set([...context1.$CURRENT_ROLE, ...context2.$CURRENT_ROLE]),
		$CURRENT_ROLES: new Set([...context1.$CURRENT_ROLES, ...context2.$CURRENT_ROLES]),
		$CURRENT_POLICIES: new Set([...context1.$CURRENT_POLICIES, ...context2.$CURRENT_POLICIES]),
	};

	return permissionContext;
}
Repository: directus/directus
File: api/src/services/graphql/schema/read.ts (TypeScript)

Code before:
					const result = await resolveQuery(gql, info);
					context['data'] = result;
					return result;
				},
			});
		}

		const eventName = `${collection.collection}_mutated`;

		if (collection.collection in ReadCollectionTypes) {
			const subscriptionType = schemaComposer.createObjectTC({
				name: eventName,
				fields: {
					key: new GraphQLNonNull(GraphQLID),
					event: subscriptionEventType,
					data: ReadCollectionTypes[collection.collection]!,
				},
			});

			schemaComposer.Subscription.addFields({
				[eventName]: {
					type: subscriptionType,
					args: {
						event: subscriptionEventType,
					},
					subscribe: await createSubscriptionGenerator(gql, eventName),
				},
			});
		}
	}

	for (const relation of schema.read.relations) {
		if (relation.related_collection) {
			if (SYSTEM_DENY_LIST.includes(relation.related_collection)) continue;

			ReadableCollectionFilterTypes[relation.collection]?.addFields({
				[relation.field]: ReadableCollectionFilterTypes[relation.related_collection]!,
			});

			ReadCollectionTypes[relation.collection]?.addFieldArgs(relation.field, {
				filter: ReadableCollectionFilterTypes[relation.related_collection]!,
				sort: {
					type: new GraphQLList(GraphQLString),
				},
				limit: {
					type: GraphQLInt,
				},
				offset: {
					type: GraphQLInt,
				},
				page: {

Review comment (suggestion):
I think this should be removed

```suggestion
					subscribe: createSubscriptionGenerator(gql, eventName),
```

Code after:
				resolve: async ({ info, context }: { info: GraphQLResolveInfo; context: Record<string, any> }) => {
					const result = await resolveQuery(gql, info);
					context['data'] = result;
					return result;
				},
			});
		}

		const eventName = `${collection.collection}_mutated`;

		if (collection.collection in ReadCollectionTypes) {
			const subscriptionType = schemaComposer.createObjectTC({
				name: eventName,
				fields: {
					key: new GraphQLNonNull(GraphQLID),
					event: subscriptionEventType,
					data: ReadCollectionTypes[collection.collection]!,
				},
			});

			schemaComposer.Subscription.addFields({
				[eventName]: {
					type: subscriptionType,
					args: {
						event: subscriptionEventType,
					},
					subscribe: createSubscriptionGenerator(gql, eventName),
				},
			});
		}
	}

	for (const collection in ReadableCollectionFilterTypes) {
		const quantifier_collection = ReadableCollectionFilterTypes[collection]?.clone(`${collection}_quantifier_filter`);

		quantifier_collection?.addFields({
			_some: ReadableCollectionFilterTypes[collection]!,
			_none: ReadableCollectionFilterTypes[collection]!,
		});

		ReadableCollectionQuantifierFilterTypes[collection] = quantifier_collection!;
	}

	for (const relation of schema.read.relations) {
		if (relation.related_collection) {
			if (SYSTEM_DENY_LIST.includes(relation.related_collection)) continue;

			ReadableCollectionQuantifierFilterTypes[relation.collection]?.addFields({
				[relation.field]: ReadableCollectionFilterTypes[relation.related_collection]!,
			});
Repository: directus/directus
File: tests/blackbox/common/functions.ts (TypeScript)

Code before:
	}

	const response = await request(getUrl(vendor))
		.post(`/policies`)
		.set('Authorization', `Bearer ${USER.TESTS_FLOW.TOKEN}`)
		.send({
			name: options.name,
			app_access: options.appAccessEnabled,
			admin_access: options.adminAccessEnabled,
			roles: [{ role: roleId }],
		});

	return response.body.data;
}

export type OptionsCreatePermission = {
	role: keyof typeof ROLE;
	permission: Omit<Partial<Permission>, 'id' | 'role' | 'system'>;
} & OptionsCreatePermissionPolicy;

export type OptionsCreatePermissionPolicy = {
	policyId?: string;
	policyName?: string;
};

export async function CreatePermissionWithPolicy(
	vendor: Vendor,
	options: OptionsCreatePermission,
): Promise<Permission> {
	let policyId = options.policyId;
	let roleId = options.role;

	if (roleId in ROLE) {
		const role = await request(getUrl(vendor))
			.get('/roles')
			.query({ filter: { name: { _eq: ROLE[roleId].NAME } } })
			.set('Authorization', `Bearer ${USER.APP_ACCESS.TOKEN}`);

		roleId = role.body.data[0].id;
	}

	if (!policyId) {
		const policy = await CreatePolicyWithRole(vendor, {
			role: roleId,
			adminAccessEnabled: false,
			appAccessEnabled: false,
			name: options.policyName ? `${options.role}-${options.policyName}` : `${options.role}-${randomUUID()}`,
		});

		policyId = policy.id;
	}

Review comment (security):
Not too sure about the renaming, `CreatePermission` should be able to account for the creation with/without policy.

Code after:
	return response.body.data;
}

export type OptionsCreatePermission = {
	role: keyof typeof ROLE;
	permission: Omit<Partial<Permission>, 'id' | 'role' | 'system'>;
	policy?: string;
	policyName?: string;
};

export async function CreatePermission(vendor: Vendor, options: OptionsCreatePermission) {
	let policyId = options.policy;
	let roleId = options.role;

	if (roleId in ROLE) {
		const role = await request(getUrl(vendor))
			.get('/roles')
			.query({ filter: { name: { _eq: ROLE[roleId].NAME } } })
			.set('Authorization', `Bearer ${USER.APP_ACCESS.TOKEN}`);

		roleId = role.body.data[0].id;
	}

	if (!policyId) {
		const policy = await CreatePolicy(vendor, {
			role: roleId,
			adminAccessEnabled: false,
			appAccessEnabled: false,
			name: options.policyName ? `${options.role}-${options.policyName}` : `${options.role}-${randomUUID()}`,
		});

		policyId = policy.id;
	}

	const response = await request(getUrl(vendor))
		.patch(`/policies/${policyId}`)
		.set('Authorization', `Bearer ${USER.TESTS_FLOW.TOKEN}`)
		.send({ permissions: { create: [{ ...options.permission, policy: options.policy }], update: [], delete: [] } });

	return response.body.data;
}

// TODO
// export async function UpdatePermission() {}
// export async function DeletePermission() {}
Repository: directus/directus
File: app/src/components/v-field-list/v-field-list.vue (Vue)

Code before:
<script setup lang="ts">
import { useFieldTree, type FieldNode } from '@/composables/use-field-tree';
import { useCollectionsStore } from '@/stores/collections';
import { useFieldsStore } from '@/stores/fields';
import { useRelationsStore } from '@/stores/relations';
import { Field } from '@directus/types';
import { debounce, isNil } from 'lodash';
import { computed, ref, toRefs, unref, watch } from 'vue';
import { useI18n } from 'vue-i18n';
import VFieldListItem from './v-field-list-item.vue';

const collectionsStore = useCollectionsStore();

const props = withDefaults(
	defineProps<{
		collection: string;
		field?: string;
		disabledFields?: string[];
		includeFunctions?: boolean;
		includeRelations?: boolean;
		includeVirtualFields?: boolean;
		relationalFieldSelectable?: boolean;
		allowSelectAll?: boolean;
		rawFieldNames?: boolean;
	}>(),
	{
		field: undefined,
		disabledFields: () => [],
		includeFunctions: false,
		includeRelations: true,
		includeVirtualFields: false,
		relationalFieldSelectable: true,
		allowSelectAll: false,
		rawFieldNames: false,
	},
);

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

const fieldsStore = useFieldsStore();
const relationsStore = useRelationsStore();

const { collection, includeRelations } = toRefs(props);

const search = ref('');


Review comment (suggestion):
Ensure consistency by following the existing pattern (app/src/interfaces/_system/system-display-template/system-display-template.vue)

```suggestion
		injectVersionField?: boolean;
```

Code after:
<script setup lang="ts">
import { useFieldTree, type FieldNode } from '@/composables/use-field-tree';
import { useCollectionsStore } from '@/stores/collections';
import { useFieldsStore } from '@/stores/fields';
import { useRelationsStore } from '@/stores/relations';
import { useFakeVersionField } from '@/composables/use-fake-version-field';
import { Field } from '@directus/types';
import { debounce, isNil } from 'lodash';
import { computed, ref, toRefs, unref, watch } from 'vue';
import { useI18n } from 'vue-i18n';
import VFieldListItem from './v-field-list-item.vue';

const collectionsStore = useCollectionsStore();

const props = withDefaults(
	defineProps<{
		collection: string;
		field?: string;
		disabledFields?: string[];
		includeFunctions?: boolean;
		includeRelations?: boolean;
		injectVersionField?: boolean;
		relationalFieldSelectable?: boolean;
		allowSelectAll?: boolean;
		rawFieldNames?: boolean;
	}>(),
	{
		field: undefined,
		disabledFields: () => [],
		includeFunctions: false,
		includeRelations: true,
		injectVersionField: false,
		relationalFieldSelectable: true,
		allowSelectAll: false,
		rawFieldNames: false,
	},
);

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

const fieldsStore = useFieldsStore();
const relationsStore = useRelationsStore();

const { collection, includeRelations } = toRefs(props);

const search = ref('');
Repository: directus/directus
File: app/src/components/v-field-list/v-field-list.vue (Vue)

Code before:
		type: 'string',
		meta: {
			id: -1,
			collection: unref(collection),
			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,
			validation: null,
			validation_message: null,
		},
	};

	return { fields: [versionField] };
});

const { treeList: treeListOriginal, loadFieldRelations, refresh } = useFieldTree(collection, virtualFields, filter);

const debouncedRefresh = debounce(() => refresh(), 250);

watch(search, () => debouncedRefresh());

const selectAllDisabled = computed(() => unref(treeList).every((field) => field.disabled === true));

const treeList = computed(() => {
	const list = treeListOriginal.value.map(setDisabled);

	if (props.field) return list.filter((fieldNode) => fieldNode.field === props.field);

	return list;

	function setDisabled(
		field: (typeof treeListOriginal.value)[number],
	): (typeof treeListOriginal.value)[number] & { disabled: boolean } {
		let disabled = field.group || false;

		if (props.disabledFields?.includes(field.key)) disabled = true;

		return {
			...field,

Review comment (refactor):
Since this is a duplicate of `injectValue` of `app/src/interfaces/_system/system-display-template/system-display-template.vue` we should reuse the code, such as through a composable.

Code after:
const debouncedRefresh = debounce(() => refresh(), 250);

watch(search, () => debouncedRefresh());

const selectAllDisabled = computed(() => unref(treeList).every((field) => field.disabled === true));

const treeList = computed(() => {
	const list = treeListOriginal.value.map(setDisabled);

	if (props.field) return list.filter((fieldNode) => fieldNode.field === props.field);

	return list;

	function setDisabled(
		field: (typeof treeListOriginal.value)[number],
	): (typeof treeListOriginal.value)[number] & { disabled: boolean } {
		let disabled = field.group || false;

		if (props.disabledFields?.includes(field.key)) disabled = true;

		return {
			...field,
			disabled,
			children: field.children?.map(setDisabled),
		};
	}
});

const addAll = () => {
	const allFields = unref(treeList).map((field) => field.field);
	emit('add', unref(allFields));
};

function filter(field: Field, parent?: FieldNode): boolean {
	if (
		!includeRelations.value &&
		(field.collection !== collection.value || (field.type === 'alias' && !field.meta?.special?.includes('group')))
	) {
		return false;
	}

	if (!search.value) {
		return true;
	}

	const children = isNil(field.schema?.foreign_key_table)
		? fieldsStore.getFieldGroupChildren(field.collection, field.field)
		: fieldsStore.getFieldsForCollection(field.schema!.foreign_key_table);

	return children?.some(matchesInNestedGroups) || matchesSearch(field) || (!!parent && matchesSearch(parent));
Repository: directus/directus
File: app/src/components/v-field-list/v-field-list.vue (Vue)

Code before:

	if (fields?.length > 10) return true;

	const anyGroupExists = fields.some((field) => field.meta?.group !== null);
	if (anyGroupExists) return true;

	const anyRelationExists = relationsStore.getRelationsForCollection(collection.value)?.length;
	if (anyRelationExists) return true;

	return false;
});

const { t } = useI18n();

const virtualFields = computed(() => {
	const collectionInfo = collectionsStore.getCollection(collection.value);
	const versioningEnabled = collectionInfo?.meta?.versioning && props.includeVirtualFields;

	if (!versioningEnabled) return null;

	const versionField: Field = {
		collection: unref(collection),
		field: '$version',
		schema: null,
		name: t('version'),
		type: 'string',
		meta: {
			id: -1,
			collection: unref(collection),
			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,
			validation: null,
			validation_message: null,
		},
	};

	return { fields: [versionField] };
});

Review comment (suggestion):
Is there a specific reason why you’ve set the type to `string` (since in the original it is an integer: `app/src/interfaces/_system/system-display-template/system-display-template.vue`)

Code after:
	if (!fields.length) return false;

	if (fields?.length > 10) return true;

	const anyGroupExists = fields.some((field) => field.meta?.group !== null);
	if (anyGroupExists) return true;

	const anyRelationExists = relationsStore.getRelationsForCollection(collection.value)?.length;
	if (anyRelationExists) return true;

	return false;
});

const { t } = useI18n();

const collectionInfo = computed(() => collectionsStore.getCollection(collection.value));
const versioningEnabled = computed(() => Boolean(collectionInfo.value?.meta?.versioning && props.injectVersionField));
const { fakeVersionField } = useFakeVersionField(collection, versioningEnabled);

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

const { treeList: treeListOriginal, loadFieldRelations, refresh } = useFieldTree(collection, injectFields, filter);

const debouncedRefresh = debounce(() => refresh(), 250);

watch(search, () => debouncedRefresh());

const selectAllDisabled = computed(() => unref(treeList).every((field) => field.disabled === true));

const treeList = computed(() => {
	const list = treeListOriginal.value.map(setDisabled);

	if (props.field) return list.filter((fieldNode) => fieldNode.field === props.field);

	return list;

	function setDisabled(
		field: (typeof treeListOriginal.value)[number],
	): (typeof treeListOriginal.value)[number] & { disabled: boolean } {
		let disabled = field.group || false;

		if (props.disabledFields?.includes(field.key)) disabled = true;

		return {
			...field,
			disabled,
			children: field.children?.map(setDisabled),
		};
	}