Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migration now allows to add required fields #54

Merged
merged 5 commits into from
Feb 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
160 changes: 150 additions & 10 deletions src/utils/field.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { MigrationOptions } from '@/src/utils/migration';
import { RONIN_SCHEMA_TEMP_SUFFIX } from '@/src/utils/misc';
import {
createFieldQuery,
Expand All @@ -6,9 +7,76 @@ import {
dropFieldQuery,
renameFieldQuery,
} from '@/src/utils/queries';
import { confirm } from '@inquirer/prompts';
import { confirm, input, select } from '@inquirer/prompts';
import type { ModelField, ModelIndex, ModelTrigger } from '@ronin/compiler';

/**
* Handles migration of a required field by prompting for a default value and generating
* the necessary queries.
* This is needed when adding a required constraint to an existing field or creating
* a new required field.
*
* @param modelSlug - The slug/identifier of the model containing the field.
* @param field - The field being made required.
* @param definedFields - The complete list of fields defined for the model.
* @param options - Optional configuration.
*
* @returns Object containing:
* - defaultValue: The chosen default value for the required field.
* - definedFields: Updated field definitions with required constraints temporarily removed.
* - queries: Array of migration queries to set default values and add required constraints.
*/
const handleRequiredField = async (
modelSlug: string,
field: ModelField,
definedFields: Array<ModelField> | undefined,
options?: MigrationOptions,
): Promise<{
defaultValue: string | boolean | undefined;
definedFields: Array<ModelField> | undefined;
queries: Array<string>;
}> => {
let defaultValue: string | boolean | undefined;
if (field.type === 'boolean') {
defaultValue =
options?.requiredDefault ||
(await select({
message: `Field ${modelSlug}.${field.slug} is required. Select a default value (or manually drop all records):`,
choices: [
{ name: 'True', value: true },
{ name: 'False', value: false },
],
}));
} else {
defaultValue =
options?.requiredDefault ||
(await input({
message: `Field ${modelSlug}.${field.slug} is required. Enter a default value (or manually drop all records):`,
}));
}

// Temporarily remove required constraints to allow setting default values.
const updatedFields = definedFields?.map((f) => ({
...f,
required: false,
}));

const queries = [
// Set the default value for all existing records.
`set.RONIN_TEMP_${modelSlug}.to({${field.slug}: ${
typeof defaultValue === 'string' ? `"${defaultValue}"` : defaultValue
}})`,
// Re-add the NOT NULL constraint after defaults are set.
`alter.model("RONIN_TEMP_${modelSlug}").alter.field("${field.slug}").to({required: true})`,
];

return {
defaultValue,
definedFields: updatedFields,
queries,
};
};

/**
* Generates the difference (migration steps) between local and remote fields of a model.
*
Expand All @@ -24,7 +92,7 @@ export const diffFields = async (
modelSlug: string,
indexes: Array<ModelIndex>,
triggers: Array<ModelTrigger>,
rename?: boolean,
options?: MigrationOptions,
): Promise<Array<string>> => {
const diff: Array<string> = [];

Expand All @@ -38,7 +106,7 @@ export const diffFields = async (
// Ask if the user wants to rename a field.
for (const field of fieldsToBeRenamed) {
const confirmRename =
rename ||
options?.rename ||
(await confirm({
message: `Did you mean to rename field: ${modelSlug}.${field.from.slug} -> ${modelSlug}.${field.to.slug}`,
default: true,
Expand Down Expand Up @@ -73,7 +141,13 @@ export const diffFields = async (
}
}

const createFieldsQueries = createFields(fieldsToAdd, modelSlug, definedFields);
const createFieldsQueries = await createFields(
fieldsToAdd,
modelSlug,
definedFields,
options,
);

diff.push(...createFieldsQueries);
if (
!(
Expand All @@ -85,12 +159,31 @@ export const diffFields = async (
}

for (const field of queriesForAdjustment || []) {
// SQLite's ALTER TABLE is limited - adding UNIQUE or NOT NULL to an existing column
// requires recreating the entire table. For other constraint changes, we can use a
// temporary column approach (create temp, copy data, drop old, rename temp).
// SQLite has limited ALTER TABLE support. When adding UNIQUE or NOT NULL constraints,
// we must recreate the entire table. For other constraint changes, we can use a more
// efficient approach: create a temporary column, copy the data, drop the old column,
// and rename the temporary one.
const existingField = existingFields.find((f) => f.slug === field.slug);
if (field.unique || field.required || existingField?.unique) {
if (field.unique || existingField?.unique) {
diff.push(...adjustFields(modelSlug, definedFields, indexes, triggers));
} else if (field.required && !field.defaultValue) {
const { definedFields: updatedFields, queries } = await handleRequiredField(
modelSlug,
field,
definedFields,
options,
);

diff.push(
...createTempModelQuery(
modelSlug,
updatedFields || [],
[],
[],
queries,
existingFields,
),
);
} else if (field.type === 'link' && field.kind === 'many') {
diff.push(...adjustFields(modelSlug, definedFields, indexes, triggers));
} else {
Expand Down Expand Up @@ -219,18 +312,41 @@ export const fieldsToCreate = (
*
* @returns An array of SQL queries for creating fields.
*/
export const createFields = (
export const createFields = async (
fields: Array<ModelField>,
modelSlug: string,
definedFields?: Array<ModelField>,
): Array<string> => {
options?: MigrationOptions,
): Promise<Array<string>> => {
const diff: Array<string> = [];

for (const fieldToAdd of fields) {
// If the field is unique, we need to create a temporary model with the existing fields
// and the new field. This is because SQLite doesn't support adding a UNIQUE constraint
// to an existing column.
if (fieldToAdd.unique) {
const existingFields = definedFields?.filter(
(f) => !fields.find((f2) => f2.slug === f.slug),
);

if (fieldToAdd.required && !fieldToAdd.defaultValue) {
const { definedFields: updatedFields, queries } = await handleRequiredField(
modelSlug,
fieldToAdd,
definedFields,
options,
);

return createTempModelQuery(
modelSlug,
updatedFields || [],
[],
[],
queries,
existingFields,
);
}

return createTempModelQuery(
modelSlug,
definedFields || [],
Expand All @@ -240,6 +356,30 @@ export const createFields = (
existingFields,
);
}
// Handle required fields by prompting for default value since SQLite doesn't allow
// adding NOT NULL columns without defaults.
if (fieldToAdd.required && !fieldToAdd.defaultValue) {
const { defaultValue } = await handleRequiredField(
modelSlug,
fieldToAdd,
definedFields,
options,
);

// Create field without NOT NULL constraint.
diff.push(createFieldQuery(modelSlug, { ...fieldToAdd, required: false }));
// Now set a placeholder value.
diff.push(
`set.${modelSlug}.to({${fieldToAdd.slug}: ${
typeof defaultValue === 'boolean' ? defaultValue : `"${defaultValue}"`
}})`,
);
// Now add the NOT NULL constraint.
diff.push(
`alter.model("${modelSlug}").alter.field("${fieldToAdd.slug}").to({required: true})`,
);
return diff;
}
diff.push(createFieldQuery(modelSlug, fieldToAdd));
}

Expand Down
18 changes: 13 additions & 5 deletions src/utils/migration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,14 @@ import {
import { confirm } from '@inquirer/prompts';
import type { Model } from '@ronin/compiler';

/**
* Options for migration operations.
*/
export type MigrationOptions = {
rename?: boolean;
requiredDefault?: boolean | string;
};

/**
* Fields to ignore.
* There are several fields that are not relevant for the migration process.
Expand Down Expand Up @@ -40,7 +48,7 @@ export const IGNORED_FIELDS = [
export const diffModels = async (
definedModels: Array<Model>,
existingModels: Array<Model>,
rename?: boolean,
options?: MigrationOptions,
): Promise<Array<string>> => {
const diff: Array<string> = [];

Expand All @@ -56,7 +64,7 @@ export const diffModels = async (
// Ask if the user wants to rename the models
for (const model of modelsToBeRenamed) {
const confirmRename =
rename ||
options?.rename ||
(process.env.NODE_ENV !== 'test' &&
(await confirm({
message: `Did you mean to rename model: ${model.from.slug} -> ${model.to.slug}`,
Expand All @@ -74,7 +82,7 @@ export const diffModels = async (
diff.push(...adjustModelMetaQueries);
diff.push(...dropModels(modelsToBeDropped));
diff.push(...createModels(modelsToBeAdded));
diff.push(...(await adjustModels(definedModels, existingModels, rename)));
diff.push(...(await adjustModels(definedModels, existingModels, options)));
diff.push(...recreateIndexes);
diff.push(...recreateTriggers);

Expand All @@ -94,7 +102,7 @@ export const diffModels = async (
const adjustModels = async (
definedModels: Array<Model>,
existingModels: Array<Model>,
rename?: boolean,
options?: MigrationOptions,
): Promise<Array<string>> => {
const diff: Array<string> = [];

Expand All @@ -109,7 +117,7 @@ const adjustModels = async (
localModel.slug,
localModel.indexes || [],
localModel.triggers || [],
rename,
options,
)),
);
}
Expand Down
49 changes: 49 additions & 0 deletions tests/fixtures/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,22 @@ export const ModelsA = [TestAccount, TestProfile, TestBlog];
export const ModelsB = [TestAccount2, TestProfile2, TestBlog2, TestComments];
/* export const ModelsC = [Account2, Profile]; */

export const AccountWithBoolean = model({
slug: 'account',
fields: {
name: string(),
email: boolean(),
},
}) as unknown as Model;

export const AccountWithRequiredBoolean = model({
slug: 'account',
fields: {
name: string(),
email: boolean({ required: true }),
},
}) as unknown as Model;

export const Account = model({
slug: 'account',
pluralSlug: 'accounts',
Expand All @@ -139,6 +155,22 @@ export const AccountNew = model({
},
}) as unknown as Model;

export const AccountWithoutUnique = model({
slug: 'account',
fields: {
name: string(),
email: string({ required: true }),
},
}) as unknown as Model;

export const AccountWithRequiredDefault = model({
slug: 'account',
fields: {
name: string(),
email: string({ required: true, defaultValue: 'RONIN_TEST_VALUE_REQUIRED_DEFAULT' }),
},
}) as unknown as Model;

export const Account2 = model({
slug: 'account',
pluralSlug: 'accounts',
Expand All @@ -150,6 +182,15 @@ export const Account2 = model({
},
}) as unknown as Model;

export const Account3 = model({
slug: 'account',
pluralSlug: 'accounts',
fields: {
name: string({ required: true, unique: true }),
email: string({ required: true, unique: true }),
},
}) as unknown as Model;

export const Profile = model({
slug: 'profile',
fields: {
Expand Down Expand Up @@ -350,3 +391,11 @@ export const TestT = model({
email: string(),
},
}) as unknown as Model;

export const TestU = model({
slug: 'test',
fields: {
test: string(),
name: string({ required: true }),
},
}) as unknown as Model;
Loading
Loading