From 025bb7c053a90ac7e1ff42b241bb300fb07967ec Mon Sep 17 00:00:00 2001
From: ian <licitdev@gmail.com>
Date: Thu, 19 Jan 2023 03:31:59 +0800
Subject: [PATCH] Fix insights filtering (#16139)

* Parse string filter as JSON

* Parse content as JSON when toggling raw editor

* Refactor missing Dashboard type

* Convert filter to gql format

* Disable alias field filter selection in insights

* Use parseJSON util

* Refactor to allow selectivity of relational field for GraphQL filters

* Emit variables

* Skip emitting invalid JSON

* Add unit tests
---
 .../v-field-list/v-field-list-item.vue        |   5 +-
 .../components/v-field-list/v-field-list.vue  |   3 +
 .../_system/system-filter/input-component.vue |   3 +-
 .../_system/system-filter/nodes.vue           |   3 +
 .../_system/system-filter/system-filter.vue   |  23 ++-
 .../system-raw-editor/system-raw-editor.vue   |  13 +-
 app/src/panels/list/index.ts                  |   1 +
 app/src/panels/metric/index.ts                |   1 +
 app/src/panels/relational-variable/index.ts   |   1 +
 app/src/panels/time-series/index.ts           |   1 +
 app/src/utils/query-to-gql-string.test.ts     | 134 ++++++++++++++++++
 app/src/utils/query-to-gql-string.ts          |  38 ++++-
 12 files changed, 211 insertions(+), 15 deletions(-)
 create mode 100644 app/src/utils/query-to-gql-string.test.ts

diff --git a/app/src/components/v-field-list/v-field-list-item.vue b/app/src/components/v-field-list/v-field-list-item.vue
index 4c5688aa37..7e29e01d8c 100644
--- a/app/src/components/v-field-list/v-field-list-item.vue
+++ b/app/src/components/v-field-list/v-field-list-item.vue
@@ -1,7 +1,7 @@
 <template>
 	<v-list-group
 		v-if="field.children || supportedFunctions.length > 0"
-		:clickable="!field.disabled"
+		:clickable="!field.disabled && (relationalFieldSelectable || !field.relatedCollection)"
 		:value="field.path"
 		@click="$emit('add', field.key)"
 	>
@@ -39,6 +39,7 @@
 			:field="childField"
 			:search="search"
 			:include-functions="includeFunctions"
+			:relational-field-selectable="relationalFieldSelectable"
 			@add="$emit('add', $event)"
 		/>
 	</v-list-group>
@@ -72,11 +73,13 @@ interface Props {
 	field: FieldInfo;
 	search?: string;
 	includeFunctions?: boolean;
+	relationalFieldSelectable?: boolean;
 }
 
 const props = withDefaults(defineProps<Props>(), {
 	search: undefined,
 	includeFunctions: false,
+	relationalFieldSelectable: true,
 });
 
 defineEmits(['add']);
diff --git a/app/src/components/v-field-list/v-field-list.vue b/app/src/components/v-field-list/v-field-list.vue
index ba58d2dc75..00bcc3013d 100644
--- a/app/src/components/v-field-list/v-field-list.vue
+++ b/app/src/components/v-field-list/v-field-list.vue
@@ -17,6 +17,7 @@
 			:field="fieldNode"
 			:search="search"
 			:include-functions="includeFunctions"
+			:relational-field-selectable="relationalFieldSelectable"
 			@add="$emit('select-field', $event)"
 		/>
 	</v-list>
@@ -37,6 +38,7 @@ interface Props {
 	disabledFields?: string[];
 	includeFunctions?: boolean;
 	includeRelations?: boolean;
+	relationalFieldSelectable?: boolean;
 }
 
 const props = withDefaults(defineProps<Props>(), {
@@ -44,6 +46,7 @@ const props = withDefaults(defineProps<Props>(), {
 	disabledFields: () => [],
 	includeFunctions: false,
 	includeRelations: true,
+	relationalFieldSelectable: true,
 });
 
 defineEmits(['select-field']);
diff --git a/app/src/interfaces/_system/system-filter/input-component.vue b/app/src/interfaces/_system/system-filter/input-component.vue
index 595f288b26..7160eefa12 100644
--- a/app/src/interfaces/_system/system-filter/input-component.vue
+++ b/app/src/interfaces/_system/system-filter/input-component.vue
@@ -158,7 +158,8 @@ export default defineComponent({
 
 			if (
 				typeof val === 'string' &&
-				['$NOW', '$CURRENT_USER', '$CURRENT_ROLE'].some((prefix) => val.startsWith(prefix))
+				(['$NOW', '$CURRENT_USER', '$CURRENT_ROLE'].some((prefix) => val.startsWith(prefix)) ||
+					/^{{\s*?\S+?\s*?}}$/.test(val))
 			) {
 				return emit('input', val);
 			}
diff --git a/app/src/interfaces/_system/system-filter/nodes.vue b/app/src/interfaces/_system/system-filter/nodes.vue
index 048ed4f32b..ae09568ec4 100644
--- a/app/src/interfaces/_system/system-filter/nodes.vue
+++ b/app/src/interfaces/_system/system-filter/nodes.vue
@@ -29,6 +29,7 @@
 								:field="field"
 								include-functions
 								:include-relations="includeRelations"
+								:relational-field-selectable="relationalFieldSelectable"
 								@select-field="updateField(index, $event)"
 							/>
 						</v-menu>
@@ -142,6 +143,7 @@ interface Props {
 	inline?: boolean;
 	includeValidation?: boolean;
 	includeRelations?: boolean;
+	relationalFieldSelectable?: boolean;
 }
 
 const props = withDefaults(defineProps<Props>(), {
@@ -150,6 +152,7 @@ const props = withDefaults(defineProps<Props>(), {
 	inline: false,
 	includeValidation: false,
 	includeRelations: true,
+	relationalFieldSelectable: true,
 });
 
 const emit = defineEmits(['remove-node', 'update:filter', 'change']);
diff --git a/app/src/interfaces/_system/system-filter/system-filter.vue b/app/src/interfaces/_system/system-filter/system-filter.vue
index 291163cbb1..8d52f3330b 100644
--- a/app/src/interfaces/_system/system-filter/system-filter.vue
+++ b/app/src/interfaces/_system/system-filter/system-filter.vue
@@ -20,6 +20,7 @@
 				:depth="1"
 				:include-validation="includeValidation"
 				:include-relations="includeRelations"
+				:relational-field-selectable="relationalFieldSelectable"
 				@remove-node="removeNode($event)"
 				@change="emitValue"
 			/>
@@ -44,6 +45,7 @@
 					:collection="collection"
 					include-functions
 					:include-relations="includeRelations"
+					:relational-field-selectable="relationalFieldSelectable"
 					@select-field="addNode($event)"
 				>
 					<template #prepend>
@@ -83,7 +85,12 @@
 import { useFieldsStore } from '@/stores/fields';
 import { useRelationsStore } from '@/stores/relations';
 import { Filter, Type, FieldFunction } from '@directus/shared/types';
-import { getFilterOperatorsForType, getOutputTypeForFunction, parseFilterFunctionPath } from '@directus/shared/utils';
+import {
+	getFilterOperatorsForType,
+	getOutputTypeForFunction,
+	parseFilterFunctionPath,
+	parseJSON,
+} from '@directus/shared/utils';
 import { cloneDeep, get, isEmpty, set } from 'lodash';
 import { computed, inject, ref } from 'vue';
 import { useI18n } from 'vue-i18n';
@@ -91,7 +98,7 @@ import Nodes from './nodes.vue';
 import { getNodeName } from './utils';
 
 interface Props {
-	value?: Record<string, any>;
+	value?: Record<string, any> | string;
 	disabled?: boolean;
 	collectionName?: string;
 	collectionField?: string;
@@ -100,6 +107,7 @@ interface Props {
 	inline?: boolean;
 	includeValidation?: boolean;
 	includeRelations?: boolean;
+	relationalFieldSelectable?: boolean;
 }
 
 const props = withDefaults(defineProps<Props>(), {
@@ -112,6 +120,7 @@ const props = withDefaults(defineProps<Props>(), {
 	inline: false,
 	includeValidation: false,
 	includeRelations: true,
+	relationalFieldSelectable: true,
 });
 
 const emit = defineEmits(['input']);
@@ -132,14 +141,16 @@ const relationsStore = useRelationsStore();
 
 const innerValue = computed<Filter[]>({
 	get() {
-		if (!props.value || isEmpty(props.value)) return [];
+		const filterValue = typeof props.value === 'string' ? parseJSON(props.value) : props.value;
 
-		const name = getNodeName(props.value);
+		if (!filterValue || isEmpty(filterValue)) return [];
+
+		const name = getNodeName(filterValue);
 
 		if (name === '_and') {
-			return cloneDeep(props.value['_and']);
+			return cloneDeep(filterValue['_and']);
 		} else {
-			return cloneDeep([props.value]);
+			return cloneDeep([filterValue]);
 		}
 	},
 	set(newVal) {
diff --git a/app/src/interfaces/_system/system-raw-editor/system-raw-editor.vue b/app/src/interfaces/_system/system-raw-editor/system-raw-editor.vue
index 14a0ade084..8a20e20303 100644
--- a/app/src/interfaces/_system/system-raw-editor/system-raw-editor.vue
+++ b/app/src/interfaces/_system/system-raw-editor/system-raw-editor.vue
@@ -11,10 +11,11 @@ import 'codemirror/addon/mode/simple';
 import { computed, onMounted, ref, watch } from 'vue';
 import { useI18n } from 'vue-i18n';
 import { mustacheMode } from './mustacheMode';
+import { parseJSON } from '@directus/shared/utils';
 
 const props = withDefaults(
 	defineProps<{
-		value?: string;
+		value?: string | object;
 		autofocus?: boolean;
 		disabled?: boolean;
 		type?: string;
@@ -85,7 +86,15 @@ onMounted(async () => {
 		codemirror.on('change', (doc, { origin }) => {
 			if (origin === 'setValue') return;
 			const content = doc.getValue();
-			emit('input', content !== '' ? content : null);
+			if (typeof props.value === 'object') {
+				try {
+					emit('input', content !== '' ? parseJSON(content) : null);
+				} catch {
+					// Skip emitting invalid JSON
+				}
+			} else {
+				emit('input', content !== '' ? content : null);
+			}
 		});
 	}
 });
diff --git a/app/src/panels/list/index.ts b/app/src/panels/list/index.ts
index 5cbe5f6f34..f88e3e9b3b 100644
--- a/app/src/panels/list/index.ts
+++ b/app/src/panels/list/index.ts
@@ -139,6 +139,7 @@ export default definePanel({
 				interface: 'system-filter',
 				options: {
 					collectionField: 'collection',
+					relationalFieldSelectable: false,
 				},
 			},
 		},
diff --git a/app/src/panels/metric/index.ts b/app/src/panels/metric/index.ts
index d33c7257e9..cf9df31296 100644
--- a/app/src/panels/metric/index.ts
+++ b/app/src/panels/metric/index.ts
@@ -180,6 +180,7 @@ export default definePanel({
 					interface: 'system-filter',
 					options: {
 						collectionField: 'collection',
+						relationalFieldSelectable: false,
 					},
 				},
 			},
diff --git a/app/src/panels/relational-variable/index.ts b/app/src/panels/relational-variable/index.ts
index a87202fed1..39a13222bd 100644
--- a/app/src/panels/relational-variable/index.ts
+++ b/app/src/panels/relational-variable/index.ts
@@ -100,6 +100,7 @@ export default definePanel({
 				interface: 'system-filter',
 				options: {
 					collectionField: 'collection',
+					relationalFieldSelectable: false,
 				},
 			},
 		},
diff --git a/app/src/panels/time-series/index.ts b/app/src/panels/time-series/index.ts
index 37c6e90a07..5dda1e6836 100644
--- a/app/src/panels/time-series/index.ts
+++ b/app/src/panels/time-series/index.ts
@@ -378,6 +378,7 @@ export default definePanel({
 				interface: 'system-filter',
 				options: {
 					collectionField: 'collection',
+					relationalFieldSelectable: false,
 				},
 			},
 		},
diff --git a/app/src/utils/query-to-gql-string.test.ts b/app/src/utils/query-to-gql-string.test.ts
new file mode 100644
index 0000000000..d320419153
--- /dev/null
+++ b/app/src/utils/query-to-gql-string.test.ts
@@ -0,0 +1,134 @@
+import { beforeEach, describe, expect, test } from 'vitest';
+
+import { formatQuery } from '@/utils/query-to-gql-string';
+
+import { setActivePinia } from 'pinia';
+import { createTestingPinia } from '@pinia/testing';
+import { Query } from '@directus/shared/types';
+
+const collectionName = 'users';
+const primaryKeyField = 'id';
+const key = 'query_abcde';
+
+beforeEach(() => {
+	setActivePinia(
+		createTestingPinia({
+			createSpy: () => (collection) => {
+				return { collection, field: primaryKeyField };
+			},
+		})
+	);
+});
+
+describe('Empty query returns the primary key', () => {
+	test.each([true, false])(`System collection: %o`, (isSystemCollection) => {
+		const query: Query = {};
+
+		const collection = isSystemCollection ? `directus_${collectionName}` : collectionName;
+		const formatted = formatQuery({ collection, key, query });
+
+		expect(formatted).toStrictEqual({
+			__aliasFor: collectionName,
+			__args: query,
+			[primaryKeyField]: true,
+		});
+	});
+});
+
+describe('Defined fields are requested', () => {
+	test.each([true, false])(`System collection: %o`, (isSystemCollection) => {
+		const query: Query = { fields: ['aaa', 'bbb', 'ccc'] };
+
+		const collection = isSystemCollection ? `directus_${collectionName}` : collectionName;
+		const formatted = formatQuery({ collection, key, query });
+
+		expect(formatted).toStrictEqual({
+			__aliasFor: collectionName,
+			__args: {},
+			aaa: true,
+			bbb: true,
+			ccc: true,
+		});
+	});
+});
+
+describe('Aggregation query without group', () => {
+	test.each([true, false])(`System collection: %o`, (isSystemCollection) => {
+		const query: Query = { aggregate: { count: ['aaa'], sum: ['bbb', 'ccc'] } };
+
+		const collection = isSystemCollection ? `directus_${collectionName}` : collectionName;
+		const formatted = formatQuery({ collection, key, query });
+
+		expect(formatted).toStrictEqual({
+			__aliasFor: `${collectionName}_aggregated`,
+			__args: {},
+			count: {
+				aaa: true,
+			},
+			sum: {
+				bbb: true,
+				ccc: true,
+			},
+		});
+	});
+});
+
+describe('Aggregation query with group', () => {
+	test.each([true, false])(`System collection: %o`, (isSystemCollection) => {
+		const query: Query = { aggregate: { count: ['aaa'], sum: ['bbb', 'ccc'] }, group: ['ddd', 'eee'] };
+
+		const collection = isSystemCollection ? `directus_${collectionName}` : collectionName;
+		const formatted = formatQuery({ collection, key, query });
+
+		expect(formatted).toStrictEqual({
+			__aliasFor: `${collectionName}_aggregated`,
+			__args: {
+				groupBy: ['ddd', 'eee'],
+			},
+			count: {
+				aaa: true,
+			},
+			sum: {
+				bbb: true,
+				ccc: true,
+			},
+			group: true,
+		});
+	});
+});
+
+describe('Filter query without functions', () => {
+	test.each([true, false])(`System collection: %o`, (isSystemCollection) => {
+		const query: Query = { filter: { _and: [{ aaa: { _eq: '111' } }, { bbb: { ccc: { _eq: '222' } } }] } };
+
+		const collection = isSystemCollection ? `directus_${collectionName}` : collectionName;
+		const formatted = formatQuery({ collection, key, query });
+
+		expect(formatted).toStrictEqual({
+			__aliasFor: collectionName,
+			__args: {
+				filter: { _and: [{ aaa: { _eq: '111' } }, { bbb: { ccc: { _eq: '222' } } }] },
+			},
+			[primaryKeyField]: true,
+		});
+	});
+});
+
+describe('Filter query with functions', () => {
+	test.each([true, false])(`System collection: %o`, (isSystemCollection) => {
+		const query: Query = {
+			filter: { _and: [{ 'count(aaa)': { _eq: '111' } }, { bbb: { 'sum(ccc)': { _eq: '222' } } }] },
+		};
+
+		const collection = isSystemCollection ? `directus_${collectionName}` : collectionName;
+		const formatted = formatQuery({ collection, key, query });
+
+		expect(formatted).toStrictEqual({
+			__aliasFor: collectionName,
+			__args: {
+				filter: { _and: [{ aaa_func: { count: { _eq: '111' } } }, { bbb: { ccc_func: { sum: { _eq: '222' } } } }] },
+			},
+			[primaryKeyField]: true,
+		});
+	});
+});
diff --git a/app/src/utils/query-to-gql-string.ts b/app/src/utils/query-to-gql-string.ts
index ac1ad7da58..c4e62d0edb 100644
--- a/app/src/utils/query-to-gql-string.ts
+++ b/app/src/utils/query-to-gql-string.ts
@@ -1,8 +1,9 @@
 import { useFieldsStore } from '@/stores/fields';
-import { Query } from '@directus/shared/types';
-import { toArray } from '@directus/shared/utils';
+import { Filter, Query } from '@directus/shared/types';
+import { parseJSON, toArray } from '@directus/shared/utils';
 import { jsonToGraphQLQuery } from 'json-to-graphql-query';
-import { isEmpty, pick, set, omitBy, isUndefined } from 'lodash';
+import { isEmpty, pick, set, omitBy, isUndefined, transform } from 'lodash';
+import { extractFieldFromFunction } from './extract-field-from-function';
 
 type QueryInfo = { collection: string; key: string; query: Query };
 
@@ -61,8 +62,8 @@ export function formatQuery({ collection, query }: QueryInfo): Record<string, an
 
 	if (query.filter) {
 		try {
-			const json = String(query.filter);
-			formattedQuery.__args.filter = JSON.parse(json);
+			const filterValue = typeof query.filter === 'object' ? query.filter : parseJSON(String(query.filter));
+			formattedQuery.__args.filter = replaceFuncs(filterValue);
 		} catch {
 			// Keep current value there
 		}
@@ -70,3 +71,30 @@ export function formatQuery({ collection, query }: QueryInfo): Record<string, an
 
 	return formattedQuery;
 }
+
+/**
+ * Replace functions from Directus-Filter format to GraphQL format
+ */
+function replaceFuncs(filter?: Filter | null): null | undefined | Filter {
+	if (!filter) return filter;
+
+	return replaceFuncDeep(filter);
+
+	function replaceFuncDeep(filter: Record<string, any>) {
+		return transform(filter, (result: Record<string, any>, value, key) => {
+			if (typeof key === 'string' && key.includes('(') && key.includes(')')) {
+				const { fn, field } = extractFieldFromFunction(key);
+
+				if (fn) {
+					result[`${field}_func`] = {
+						[fn]: value,
+					};
+				} else {
+					result[key] = value;
+				}
+			} else {
+				result[key] = value?.constructor === Object || value?.constructor === Array ? replaceFuncDeep(value) : value;
+			}
+		});
+	}
+}