Ask for value when changing nullable to not-nullable (#5400)

* Add ContainsNullValues exception abstraction

* Add dialog for null values when disabling non-null

Fixes #2934

* Add translation for CONTAINS_NULL_VALUE error

* Make dialog title translated

api/src/exceptions/database/contains-null-values.ts

@@ -0,0 +1,12 @@
+import { BaseException } from '../base';
+
+type Exceptions = {
+	collection: string;
+	field: string;
+};
+
+export class ContainsNullValuesException extends BaseException {
+	constructor(field: string, exceptions?: Exceptions) {
+		super(`Field "${field}" contains null values.`, 400, 'CONTAINS_NULL_VALUES', exceptions);
+	}
+}

api/src/exceptions/database/dialects/mssql.ts

@@ -1,4 +1,5 @@
 import database from '../../../database';
+import { ContainsNullValuesException } from '../contains-null-values';
 import { InvalidForeignKeyException } from '../invalid-foreign-key';
 import { NotNullViolationException } from '../not-null-violation';
 import { RecordNotUniqueException } from '../record-not-unique';
@@ -134,6 +135,10 @@ function notNullViolation(error: MSSQLError) {
 	const collection = bracketMatches[0].slice(1, -1);
 	const field = quoteMatches[0].slice(1, -1);
 
+	if (error.message.includes('Cannot insert the value NULL into column')) {
+		return new ContainsNullValuesException(field, { collection, field });
+	}
+
 	return new NotNullViolationException(field, {
 		collection,
 		field,

api/src/exceptions/database/dialects/mysql.ts

@@ -1,3 +1,4 @@
+import { ContainsNullValuesException } from '../contains-null-values';
 import { InvalidForeignKeyException } from '../invalid-foreign-key';
 import { NotNullViolationException } from '../not-null-violation';
 import { RecordNotUniqueException } from '../record-not-unique';
@@ -11,6 +12,8 @@ enum MySQLErrorCodes {
 	ER_DATA_TOO_LONG = 'ER_DATA_TOO_LONG',
 	NOT_NULL_VIOLATION = 'ER_BAD_NULL_ERROR',
 	FOREIGN_KEY_VIOLATION = 'ER_NO_REFERENCED_ROW_2',
+	ER_INVALID_USE_OF_NULL = 'ER_INVALID_USE_OF_NULL',
+	WARN_DATA_TRUNCATED = 'WARN_DATA_TRUNCATED',
 }
 
 export function extractError(error: MySQLError): MySQLError | Error {
@@ -25,7 +28,12 @@ export function extractError(error: MySQLError): MySQLError | Error {
 			return notNullViolation(error);
 		case MySQLErrorCodes.FOREIGN_KEY_VIOLATION:
 			return foreignKeyViolation(error);
+		// Note: MariaDB throws data truncated for null value error
+		case MySQLErrorCodes.ER_INVALID_USE_OF_NULL:
+		case MySQLErrorCodes.WARN_DATA_TRUNCATED:
+			return containsNullValues(error);
 	}
+
 	return error;
 }
 
@@ -132,3 +140,17 @@ function foreignKeyViolation(error: MySQLError) {
 		invalid,
 	});
 }
+
+function containsNullValues(error: MySQLError) {
+	const betweenTicks = /`([^`]+)`/g;
+
+	// Normally, we shouldn't read from the executed SQL. In this case, we're altering a single
+	// column, so we shouldn't have the problem where multiple columns are altered at the same time
+	const tickMatches = error.sql.match(betweenTicks);
+
+	if (!tickMatches) return error;
+
+	const field = tickMatches[1].slice(1, -1);
+
+	return new ContainsNullValuesException(field);
+}

api/src/exceptions/database/dialects/oracle.ts

@@ -1,3 +1,28 @@
-export function extractError(error: Error): Error {
-	return error;
+import { ContainsNullValuesException } from '../contains-null-values';
+import { OracleError } from './types';
+
+enum OracleErrorCodes {
+	'CONTAINS_NULL_VALUES' = 2296,
+	// @TODO extend with other errors
+}
+
+export function extractError(error: OracleError): OracleError | Error {
+	switch (error.errorNum) {
+		case OracleErrorCodes.CONTAINS_NULL_VALUES:
+			return containsNullValues(error);
+		default:
+			return error;
+	}
+}
+
+function containsNullValues(error: OracleError): OracleError | ContainsNullValuesException {
+	const betweenQuotes = /"([^"]+)"/g;
+	const matches = error.message.match(betweenQuotes);
+
+	if (!matches) return error;
+
+	const collection = matches[0].slice(1, -1);
+	const field = matches[1].slice(1, -1);
+
+	return new ContainsNullValuesException(field, { collection, field });
 }

api/src/exceptions/database/dialects/postgres.ts

@@ -1,3 +1,4 @@
+import { ContainsNullValuesException } from '../contains-null-values';
 import { InvalidForeignKeyException } from '../invalid-foreign-key';
 import { NotNullViolationException } from '../not-null-violation';
 import { RecordNotUniqueException } from '../record-not-unique';
@@ -88,9 +89,12 @@ function valueLimitViolation(error: PostgresError) {
 
 function notNullViolation(error: PostgresError) {
 	const { table, column } = error;
-
 	if (!column) return error;
 
+	if (error.message.endsWith('contains null values')) {
+		return new ContainsNullValuesException(column, { collection: table, field: column });
+	}
+
 	return new NotNullViolationException(column, {
 		collection: table,
 		field: column,

api/src/exceptions/database/dialects/sqlite.ts

@@ -1,3 +1,4 @@
+import { ContainsNullValuesException } from '../contains-null-values';
 import { InvalidForeignKeyException } from '../invalid-foreign-key';
 import { NotNullViolationException } from '../not-null-violation';
 import { RecordNotUniqueException } from '../record-not-unique';
@@ -41,6 +42,16 @@ function notNullConstraint(error: SQLiteError) {
 	const [table, column] = errorParts[errorParts.length - 1].split('.');
 
 	if (table && column) {
+		// Now this gets a little finicky... SQLite doesn't have any native ALTER, so Knex implements
+		// it by creating a new table, and then copying the data over. That also means we'll never get
+		// a ContainsNullValues constraint error, as there is no ALTER. HOWEVER, we can hack around
+		// that by checking for the collection name, as Knex's alter default template name will always
+		// start with _knex_temp. The best we can do in this case is check for that, and use it to
+		// decide between NotNullViolation and ContainsNullValues
+		if (table.startsWith('_knex_temp_alter')) {
+			return new ContainsNullValuesException(column);
+		}
+
 		return new NotNullViolationException(column, {
 			collection: table,
 			field: column,

api/src/exceptions/database/dialects/types.ts

@@ -31,10 +31,16 @@ export type PostgresError = {
 	constraint?: string;
 };
 
+export type OracleError = {
+	message: string;
+	errorNum: number;
+	offset: number;
+};
+
 export type SQLiteError = {
 	message: string;
 	errno: number;
 	code: string;
 };
 
-export type SQLError = MSSQLError & MySQLError & PostgresError & SQLiteError & Error;
+export type SQLError = MSSQLError & MySQLError & PostgresError & SQLiteError & OracleError & Error;

api/src/services/fields.ts

@@ -8,6 +8,7 @@ import { systemFieldRows } from '../database/system-data/fields/';
 import emitter, { emitAsyncSafe } from '../emitter';
 import env from '../env';
 import { ForbiddenException, InvalidPayloadException } from '../exceptions';
+import { translateDatabaseError } from '../exceptions/database/translate';
 import { ItemsService } from '../services/items';
 import { PayloadService } from '../services/payload';
 import { AbstractServiceOptions, Accountability, FieldMeta, SchemaOverview, types } from '../types';
@@ -253,10 +254,15 @@ export class FieldsService {
 
 		if (field.schema) {
 			const existingColumn = await this.schemaInspector.columnInfo(collection, field.field);
-			await this.knex.schema.alterTable(collection, (table) => {
-				if (!field.schema) return;
-				this.addColumnToTable(table, field, existingColumn);
-			});
+
+			try {
+				await this.knex.schema.alterTable(collection, (table) => {
+					if (!field.schema) return;
+					this.addColumnToTable(table, field, existingColumn);
+				});
+			} catch (err) {
+				throw await translateDatabaseError(err);
+			}
 		}
 
 		if (field.meta) {

app/src/lang/translations/en-US.yaml

@@ -91,6 +91,7 @@ no_access: No Access
 use_custom: Use Custom
 nullable: Nullable
 allow_null_value: Allow NULL value
+enter_value_to_replace_nulls: Please enter a new value to replace any NULLs currently within this field.
 field_standard: Standard
 field_presentation: Presentation & Aliases
 field_file: Single File
@@ -408,6 +409,7 @@ errors:
   ROUTE_NOT_FOUND: Not found
   RECORD_NOT_UNIQUE: Duplicate value detected
   USER_SUSPENDED: User Suspended
+  CONTAINS_NULL_VALUES: Field contains null values
   UNKNOWN: Unexpected Error
 value_hashed: Value Securely Hashed
 bookmark_name: Bookmark name...

app/src/modules/settings/routes/data-model/field-detail/field-detail.vue

@@ -96,6 +96,21 @@
 				@cancel="cancelField"
 			/>
 		</template>
+
+		<v-dialog v-model="nullValuesDialog" @esc="nullValuesDialog = false">
+			<v-card>
+				<v-card-title>{{ $t('enter_value_to_replace_nulls') }}</v-card-title>
+				<v-card-text>
+					<v-input placeholder="NULL" v-model="nullValueOverride" />
+				</v-card-text>
+				<v-card-actions>
+					<v-button secondary @click="nullValuesDialog = false">{{ $t('cancel') }}</v-button>
+					<v-button :disabled="nullValueOverride === null" @click="saveNullOverride" :loading="nullOverrideSaving">
+						{{ $t('save') }}
+					</v-button>
+				</v-card-actions>
+			</v-card>
+		</v-dialog>
 	</v-drawer>
 </template>
 
@@ -161,6 +176,8 @@ export default defineComponent({
 		const { collection } = toRefs(props);
 		const { info: collectionInfo } = useCollection(collection);
 
+		const { nullValueOverride, nullValuesDialog, nullOverrideSaving, saveNullOverride } = useContainsNull();
+
 		const existingField = computed(() => {
 			if (props.field === '+') return null;
 
@@ -207,6 +224,10 @@ export default defineComponent({
 			translationsManual,
 			currentTabInfo,
 			title,
+			nullValuesDialog,
+			nullValueOverride,
+			nullOverrideSaving,
+			saveNullOverride,
 		};
 
 		function useTabs() {
@@ -400,7 +421,12 @@ export default defineComponent({
 				router.push(`/settings/data-model/${props.collection}`);
 				clearLocalStore();
 			} catch (err) {
-				unexpectedError(err);
+				if (err?.response?.data?.errors?.[0]?.extensions?.code === 'CONTAINS_NULL_VALUES') {
+					nullValueOverride.value = state.fieldData?.schema?.default_value || null;
+					nullValuesDialog.value = true;
+				} else {
+					unexpectedError(err);
+				}
 			} finally {
 				saving.value = false;
 			}
@@ -410,6 +436,49 @@ export default defineComponent({
 			router.push(`/settings/data-model/${props.collection}`);
 			clearLocalStore();
 		}
+
+		/**
+		 * In case you're setting allow null to false and you have null values already stored, we need
+		 * to override those null values with a new value before you can try saving again
+		 */
+		function useContainsNull() {
+			const nullValuesDialog = ref(false);
+			const nullValueOverride = ref();
+			const nullOverrideSaving = ref(false);
+
+			return { nullValueOverride, nullValuesDialog, nullOverrideSaving, saveNullOverride };
+
+			async function saveNullOverride() {
+				nullOverrideSaving.value = true;
+
+				try {
+					const endpoint = props.collection.startsWith('directus_')
+						? `/${props.collection.substring(9)}`
+						: `/items/${props.collection}`;
+
+					await api.patch(endpoint, {
+						query: {
+							filter: {
+								[props.field]: {
+									_null: true,
+								},
+							},
+							limit: -1,
+						},
+						data: {
+							[props.field]: nullValueOverride.value,
+						},
+					});
+
+					nullValuesDialog.value = false;
+					return saveField();
+				} catch (err) {
+					unexpectedError(err);
+				} finally {
+					nullOverrideSaving.value = false;
+				}
+			}
+		}
 	},
 });
 </script>