Default value for boolean fields set as String

[eikaramba]

Describe the Bug

i have a boolean input field where i set the default value to false

then i create a new entry without the boolean input (which should be set to the default value false)

However the field is created as a string "false" not false

To Reproduce

1. create toggle field with default value false
2. post new collection entry without the toggle field
3. see response object

Errors Shown

No response

What version of Directus are you using?

9.14.1

What version of Node.js are you using?

16

What database are you using?

SQLITE

What browser are you using?

FF

How are you deploying Directus?

local

[eikaramba]

interestingly this is what i see in the database. all other values with 0/1 are displayed correctly and are fine in the directus app as well. only the new test field with a value of false is also not correctly shown in the directus app (e.g. it has a checkmark in the overview)

this is the schema of the table

[eikaramba]

Setting the default value to 0 manually in the sqlite database schema fixes everything. so i would argue that the default value set by directus is wrong

[azrikahar]

Initially I wasn't able to reproduce this:

chrome_Bem4g3y8T4.mp4

But after doing further testings, it seems like it exclusively happens AFTER updating the default value back to false, but not when we initially set it as false. Here's another clip to show this clearer:

chrome_J9prCn41KJ.mp4

[aidenfoxx]

@azrikahar Feels like it could be similar to the issue addressed here knex/knex-schema-inspector#117. When updating a column to nullable in most DB vendors, it tends to return string null as the default. We should verify boolean cases for all DB vendors.

[azrikahar]

@azrikahar Feels like it could be similar to the issue addressed here knex/knex-schema-inspector#117. When updating a column to nullable in most DB vendors, it tends to return string null as the default. We should verify boolean cases for all DB vendors.

That's a good point. I haven't dig into this too much, but would this portion of the code already be the verification you're mentioning? 🤔 (which makes this even more odd on what regression could be happening, or was this never caught in the first place)

directus/api/src/utils/get-default-value.ts

Lines 32 to 42 in 9e387fe

	function castToBoolean(value: any): boolean {
	if (typeof value === 'boolean') return value;
	if (value === 0 || value === '0') return false;
	if (value === 1 || value === '1') return true;
	if (value === 'false' || value === false) return false;
	if (value === 'true' || value === true) return true;
	return Boolean(value);
	}

[aidenfoxx]

@azrikahar I didn't realize there was already explicit boolean handling in that helper, but yes, that should handle this case. It could be that the helper doesn't account for whitespace? I know it was possible for nullable values to be returned as 'null ' from the schema query.

Either way, I personally don't like that this is handled in a helper function in Directus. It's my opinion that knex-schema-inspector should handle this as part of it's parseDefaultValue function. But we can worry about that later!

[azrikahar]

Looked into this a bit more, and I'm not sure if this is accurate, but here are my thoughts:

The API does seem to correctly pass false (boolean) as default value here in the last else statement:

directus/api/src/services/fields.ts

Lines 586 to 602 in a803331

	if (field.schema?.default_value !== undefined) {
	if (
	typeof field.schema.default_value === 'string' &&
	(field.schema.default_value.toLowerCase() === 'now()' || field.schema.default_value === 'CURRENT_TIMESTAMP')
	) {
	column.defaultTo(this.knex.fn.now());
	} else if (
	typeof field.schema.default_value === 'string' &&
	field.schema.default_value.includes('CURRENT_TIMESTAMP(') &&
	field.schema.default_value.includes(')')
	) {
	const precision = field.schema.default_value.match(REGEX_BETWEEN_PARENS)![1];
	column.defaultTo(this.knex.fn.now(Number(precision)));
	} else {
	column.defaultTo(field.schema.default_value);
	}
	}

But it seems like the defaultTo in Knex sqlite alterColumn() here is not formatted correctly: https://github.com/knex/knex/blob/2dadde4214d9ee333adccfa517089647e94d23be/lib/dialects/sqlite3/schema/ddl.js#L102-L109

which may explain why this default value bug only happens when updating it, not when creating it.

Not sure if the formatDefault here was not ran somewhere before alterColumn(): https://github.com/knex/knex/blob/2dadde4214d9ee333adccfa517089647e94d23be/lib/formatter/formatterUtils.js#L21-L36, or it's not formatting properly.

Would love to hear from @nickrum since I'm still not entirely sure here.

[nickrum]

Thanks for taking a look @azrikahar. formatDefault() is called over here before being passed to SQLite3_DDL. But it looks like it is called with the wrong type name. SQL uses boolean as the type name, while Knex internally uses bool.

Changing that line from

formatDefault(col.modified['defaultTo'][0], type, this.client)

to

formatDefault(col.modified['defaultTo'][0], col.type, this.client)

fixed the issue for me.

[nickrum]

This will be fixed by knex/knex#5319.

[rijkvanzanten]

Linear: ENG-231