Bugs with the new filter interface and api

[Oreilles]

Preflight Checklist

• I have completed all Troubleshooting Steps.
• I'm on the latest version of Directus.
• There's no other issue that already describes my problem.

Describe the Bug

1. Every time I click on a field name in the dropdown to add a filter element, the error Uncaught TypeError: e.path is undefined logs to the console. Fixed
2. When clicking on a relational field in that dropdown (not on its right arrow to expand it), the same error occurs, and the element isn't added to the filter list Fixed
3. When adding any of the filters null, nnull, empty, nempty, the layout crashes with [INVALID_QUERY] "_empty" has to be a boolean Fixed
4. When setting a filter on a geometry field, the layout crashes with [INVALID_QUERY] The filter value for "_eq" has to be a string, number, or boolean Somewhat fixed
5. When setting an equal filter on a UUID field and typing a non-uuid-like string, the server throw an unhandled error with: invalid input syntax for type uuid: "justtypeanything" Fixed
6. That same error occurs as well if you try to compare a number field to a string that cannot be parsed down to a number. I only tested for these two types, but I assume that a similar error will occur for any bad type casting. Fixed

To Reproduce

Follow steps above.

What version of Directus are you using?

9.0.0-rc.96

What version of Node.js are you using?

16.8.0

What database are you using?

Postgres

What browser are you using?

Firefox

What operating system are you using?

macOS

How are you deploying Directus?

local

[licitdev]

The same bunch of errors for me too. 👀

Every time I click on a field name in the dropdown to add a filter element, the error Uncaught TypeError: e.path is undefined logs to the console.

On Firefox & Safari, the e.path is missing from the v-click-outside event.
Error can be suppressed by adding !e.path || here, but on the affected browsers, have to click on filter button again to close the filter view.

directus/app/src/views/private/components/search-input/search-input.vue

Line 109 in fc87c0b

if (e.path.some((el) => el?.classList?.contains('v-menu-content'))) return false;

Slowly looking into the other issues listed...

[nordcart]

Same here:

[azrikahar]

• When adding any of the filters null, nnull, empty, nempty, the layout crashes with [INVALID_QUERY] "_empty" has to be a boolean

directus/app/src/interfaces/_system/system-filter/nodes.vue

Lines 244 to 253 in fc87c0b

	if (['_in', '_nin'].includes(newVal)) {
	if (Array.isArray(value) === false) update([value]);
	} else if (['_between', '_nbetween'].includes(newVal)) {
	if (Array.isArray(value) && value.length >= 2) update([value[0], value[1]]);
	else update([null, null]);
	} else if (Array.isArray(value) && value.length > 0) {
	update(value[0]);
	} else {
	update(value);
	}

Seems like adding the following to the above if else statement will resolve this (may need more confirmation):

                      else if (['_null', '_nnull', '_empty', '_nempty'].includes(newVal)) {
		          update(true);
		      }

• When setting an equal filter on a UUID field and typing a non-uuid-like string, the server throw an unhandled error with: invalid input syntax for type uuid: "justtypeanything"

This should be related to #7502 right?

[licitdev]

@azrikahar it does clear the error and work. 🤓
But when switching to another operator such as equals that has a text field, true is set in that value field.

Somehow thereafter, the text field is clipped to 2 characters, think it is set to the -- placeholder when empty.

[azrikahar]

@licitdev thanks for testing it! That's why I (luckily) mentioned need more confirmation 🙈

Also thanks a lot for looking into this and sharing updates/details on what you found. Definitely helps a ton!

[licitdev]

When setting an equal filter on a UUID field and typing a non-uuid-like string, the server throw an unhandled error with: invalid input syntax for type uuid: "justtypeanything"

@azrikahar I think this issue could be solved through the app. Not sure if it's good to update the API as well.
I am thinking that a fix should be creating a updateValue() so that the data can be parsed based on the field's type.

On a side note, I wonder if debounce will be useful here in reducing the number of server requests after almost every keystroke. 🤔

[licitdev]

Just saw the following reference for composedPath(). https://developer.mozilla.org/en-US/docs/Web/API/Event/composedPath
Tested below code to work on Chrome, Safari, and FireFox.

function onClickOutside(e: any) {
	if (e.composedPath().some((el) => el?.classList?.contains('v-menu-content'))) return false;

	return true;
}

[azrikahar]

@azrikahar I think this issue could be solved through the app. Not sure if it's good to update the API as well. I am thinking that a fix should be creating a updateValue() so that the data can be parsed based on the field's type.

Hmm could be, but I'm also not super sure as I haven't look into this deeply atm haha. I believe the intention to update the API (assuming you meant that linked PR) was to ensure other clients like the SDK, not just the App, will have the same outcome. Just a very high level assumption from me here 🤔

On a side note, I wonder if debounce will be useful here in reducing the number of server requests after almost every keystroke. 🤔

Does #8631 addresses this? Feel free to let me know if I missed other inputs in that PR 👍

Just saw the following reference for composedPath(). https://developer.mozilla.org/en-US/docs/Web/API/Event/composedPath
Tested below code to work on Chrome, Safari, and FireFox.

Nice find! Seems like that is indeed the better solution going forward, as mentioned in the beginning of this SO answer, especially since Directus won't support IE anyway

[Oreilles]

• When setting an equal filter on a UUID field and typing a non-uuid-like string, the server throw an unhandled error with: invalid input syntax for type uuid: "justtypeanything"

This should be related to #7502 right?

That issue occured with the search field.
@Nitwel fixed it by handling the error in getDBQuery, which calls applyQuery, which calls applySearch, which validate the search string for each field

directus/api/src/utils/apply-query.ts

Lines 524 to 533 in f64a5be

	dbQuery.andWhere(function () {
	fields.forEach(([name, field]) => {
	if (['text', 'string'].includes(field.type)) {
	this.orWhereRaw(`LOWER(??) LIKE ?`, [`${collection}.${name}`, `%${searchQuery.toLowerCase()}%`]);
	} else if (['bigInteger', 'integer', 'decimal', 'float'].includes(field.type)) {
	const number = Number(searchQuery);
	if (!isNaN(number)) this.orWhere({ [`${collection}.${name}`]: number });
	} else if (field.type === 'uuid' && validate(searchQuery)) {
	this.orWhere({ [`${collection}.${name}`]: searchQuery });
	}

For filters, the check is done is validateFilter, which doesn't use the field type to validate the filter value. It only uses the filter type. So when a geometry field is compared with eq, this happens:

directus/api/src/utils/validate-query.ts

Lines 99 to 104 in f64a5be

	if (
	(typeof value === 'string' || typeof value === 'number' || typeof value === 'boolean' || value instanceof Date) ===
	false
	) {
	throw new InvalidQueryException(`The filter value for "${key}" has to be a string, number, or boolean`);
	}

Hence error n°3 in the bullet list.

@azrikahar I think this issue could be solved through the app. Not sure if it's good to update the API as well.

Strongly agree with this. The database will already throw if the compare value is invalid, so doing the check on Directus side as well seems unnecessary. We would just need to make sure that the App sends correct compare values to the API.

I am thinking that a fix should be creating a updateValue() so that the data can be parsed based on the field's type.

I think this should happen at the interface level. For example, when using the input to type a number, no value should be emitted if the input contains non decimal characters. Actually, we shouldn't even be able to type them. Same goes for the uuid, where we'd need a mechanism in the interface-input that validate the input value (a regex should be enough) and doesn't emit if the check fail.

On a side note, I wonder if debounce will be useful here in reducing the number of server requests after almost every keystroke. 🤔

Totally agree with this 👍 When searching on a massive collection with lots of columns, the UI just get janked and I'm pretty sure the server doesn't like it either.

[licitdev]

Hmm could be, but I'm also not super sure as I haven't look into this deeply atm haha. I believe the intention to update the API (assuming you meant that linked PR) was to ensure other clients like the SDK, not just the App, will have the same outcome. Just a very high level assumption from me here 🤔

I believe even after validating on API, an error will still be thrown, albeit a different error type.
Hence the validation should be added in the App to prevent any API requests sent for invalid inputs.

Does #8631 addresses this? Feel free to let me know if I missed other inputs in that PR 👍

Yep! The debouncing worked great~ 👍🏻

Nice find! Seems like that is indeed the better solution going forward, as mentioned in the beginning of this SO answer, especially since Directus won't support IE anyway

Yes, but the answer may not be correct as mentioned in this comment.
Perhaps if you want, you may also add in the e.path such as below.

function onClickOutside(e: any) {
	const path = e.path || e.composedPath();
	if (path.some((el) => el?.classList?.contains('v-menu-content'))) return false;

	return true;
}

[licitdev]

Whoever working on this issue, please help to add in background-color: var(--background-page);. Thanks!

directus/app/src/interfaces/_system/system-filter/input-component.vue

Lines 129 to 133 in fc87c0b

	input {
	color: var(--primary);
	font-family: var(--family-monospace);
	line-height: 1em;
	border: none;

[azrikahar]

@Oreilles thanks for the detailed explanation!

I think this should happen at the interface level. For example, when using the input to type a number, no value should be emitted if the input contains non decimal characters. Actually, we shouldn't even be able to type them. Same goes for the uuid, where we'd need a mechanism in the interface-input that validate the input value (a regex should be enough) and doesn't emit if the check fail.

Totally agree with this as well.

---

I believe even after validating on API, an error will still be thrown, albeit a different error type.
Hence the validation should be added in the App to prevent any API requests sent for invalid inputs.

@licitdev that's true. Preventing invalid inputs as Oreilles mentioned above does make more sense in general.

[agussaputrasijabat]

also on activity feed