feat: Add AlterField Operation to Migration Generator

Author: kumarUjjawal

cot-cli/src/migration_generator.rs

@@ -716,9 +716,9 @@ impl MigrationOperationGenerator {
 
     #[must_use]
     fn make_alter_field_operation(
-        _app_model: &ModelInSource,
+        app_model: &ModelInSource,
         app_field: &Field,
-        migration_model: &ModelInSource,
+        _migration_model: &ModelInSource,
         migration_field: &Field,
     ) -> Option<DynOperation> {
         if app_field == migration_field {
@@ -728,20 +728,15 @@ impl MigrationOperationGenerator {
             StatusType::Modifying,
             &format!(
                 "Field '{}' from Model '{}'",
-                &migration_field.name, migration_model.model.name
-            ),
-        );
-
-        todo!();
-
-        #[expect(unreachable_code)]
-        print_status_msg(
-            StatusType::Modified,
-            &format!(
-                "Field '{}' from Model '{}'",
-                &migration_field.name, migration_model.model.name
+                &migration_field.name, app_model.model.name
             ),
         );
+        Some(DynOperation::AlterField {
+            table_name: app_model.model.table_name.clone(),
+            model_ty: app_model.model.resolved_ty.clone(),
+            old_field: Box::new(migration_field.clone()),
+            new_field: Box::new(app_field.clone()),
+        })
     }
 
     #[must_use]
@@ -1009,24 +1004,22 @@ impl GeneratedMigration {
             } => {
                 let to_type = match to {
                     DynOperation::CreateModel { model_ty, .. } => model_ty,
-                    DynOperation::AddField { .. } => {
-                        unreachable!(
-                            "AddField operation shouldn't be a dependency of CreateModel \
-                            because it doesn't create a new model"
-                        )
-                    }
-                    DynOperation::RemoveField { .. } => {
-                        unreachable!(
-                            "RemoveField operation shouldn't be a dependency of CreateModel \
-                        because it doesn't create a new model"
-                        )
-                    }
-                    DynOperation::RemoveModel { .. } => {
-                        unreachable!(
-                            "RemoveModel operation shouldn't be a dependency of CreateModel \
-                        because it doesn't create a new model"
-                        )
-                    }
+                    DynOperation::AddField { .. } => unreachable!(
+                        "AddField operation shouldn't be a dependency of CreateModel \
+                    because it doesn't create a new model"
+                    ),
+                    DynOperation::RemoveField { .. } => unreachable!(
+                        "RemoveField operation shouldn't be a dependency of CreateModel \
+                    because it doesn't create a new model"
+                    ),
+                    DynOperation::RemoveModel { .. } => unreachable!(
+                        "RemoveModel operation shouldn't be a dependency of CreateModel \
+                    because it doesn't create a new model"
+                    ),
+                    DynOperation::AlterField { .. } => unreachable!(
+                        "AlterField operation shouldn't be a dependency of CreateModel \
+                    because it doesn't create a new model"
+                    ),
                 };
                 trace!(
                     "Removing foreign keys from {} to {}",
@@ -1063,6 +1056,11 @@ impl GeneratedMigration {
                 // RemoveModel doesn't create dependencies, it only removes a model
                 unreachable!("RemoveModel operation should never create cycles")
             }
+            DynOperation::AlterField { .. } => {
+                // AlterField only changes metadata of an existing field,
+                // so it does not create dependency cycles.
+                unreachable!("AlterField operation should never create cycles")
+            }
         }
     }
 
@@ -1161,6 +1159,18 @@ impl GeneratedMigration {
                     // RemoveField Doesnt Add Foreign Keys
                     Vec::new()
                 }
+                DynOperation::AlterField {
+                    new_field,
+                    model_ty,
+                    ..
+                } => {
+                    let mut ops = vec![(i, model_ty.clone())];
+                    // Only depend on the new foreign key, not the old one
+                    if let Some(to_type) = foreign_key_for_field(new_field) {
+                        ops.push((i, to_type));
+                    }
+                    ops
+                }
                 DynOperation::RemoveModel { .. } => {
                     // RemoveModel Doesnt Add Foreign Keys
                     Vec::new()
@@ -1317,6 +1327,12 @@ pub enum DynOperation {
         model_ty: syn::Type,
         fields: Vec<Field>,
     },
+    AlterField {
+        table_name: String,
+        model_ty: syn::Type,
+        old_field: Box<Field>,
+        new_field: Box<Field>,
+    },
 }
 
 /// Returns whether given [`Field`] is a foreign key to given type.
@@ -1371,6 +1387,22 @@ impl Repr for DynOperation {
                         .build()
                 }
             }
+            Self::AlterField {
+                table_name,
+                old_field,
+                new_field,
+                ..
+            } => {
+                let old_field = old_field.repr();
+                let new_field = new_field.repr();
+                quote! {
+                    ::cot::db::migrations::Operation::alter_field()
+                        .table_name(::cot::db::Identifier::new(#table_name))
+                        .old_field(#old_field)
+                        .new_field(#new_field)
+                        .build()
+                }
+            }
             Self::RemoveModel {
                 table_name, fields, ..
             } => {
@@ -2053,4 +2085,116 @@ mod tests {
             panic!("Expected a function item");
         }
     }
+
+    #[test]
+    fn make_alter_field_operation() {
+        let migration_model = get_test_model();
+        let mut app_model = migration_model.clone();
+
+        app_model.model.fields[0].ty = parse_quote!(i32);
+
+        let migration_field = &migration_model.model.fields[0];
+        let app_field = &app_model.model.fields[0];
+
+        let operation = MigrationOperationGenerator::make_alter_field_operation(
+            &app_model,
+            app_field,
+            &migration_model,
+            migration_field,
+        );
+
+        match &operation {
+            Some(DynOperation::AlterField {
+                table_name,
+                model_ty,
+                old_field,
+                new_field,
+            }) => {
+                assert_eq!(table_name, "test_model");
+                assert_eq!(model_ty, &parse_quote!(TestModel));
+                assert_eq!(old_field.column_name, "field1");
+                assert_eq!(old_field.ty, parse_quote!(String));
+                assert_eq!(new_field.column_name, "field1");
+                assert_eq!(new_field.ty, parse_quote!(i32));
+            }
+            _ => panic!("Expected Some(DynOperation::AlterField)"),
+        }
+    }
+
+    #[test]
+    fn generate_operations_with_altered_field() {
+        let migration_model = get_test_model();
+        let mut app_model = migration_model.clone();
+
+        app_model.model.fields[0].ty = parse_quote!(i32);
+
+        let app_models = vec![app_model.clone()];
+        let migration_models = vec![migration_model.clone()];
+
+        let (modified_models, operations) =
+            MigrationGenerator::generate_operations(&app_models, &migration_models);
+
+        assert_eq!(modified_models.len(), 1);
+        assert!(
+            operations.iter().any(|op| match op {
+                DynOperation::AlterField {
+                    old_field,
+                    new_field,
+                    ..
+                } => old_field.ty == parse_quote!(String) && new_field.ty == parse_quote!(i32),
+                _ => false,
+            }),
+            "Expected an AlterField operation for changed type"
+        );
+    }
+
+    #[test]
+    fn repr_for_alter_field_operation() {
+        let op = DynOperation::AlterField {
+            table_name: "test_table".to_string(),
+            model_ty: parse_quote!(TestModel),
+            old_field: Box::new(Field {
+                name: format_ident!("test_field"),
+                column_name: "test_field".to_string(),
+                ty: parse_quote!(String),
+                auto_value: false,
+                primary_key: false,
+                unique: false,
+                foreign_key: None,
+            }),
+            new_field: Box::new(Field {
+                name: format_ident!("test_field"),
+                column_name: "test_field".to_string(),
+                ty: parse_quote!(i32),
+                auto_value: false,
+                primary_key: false,
+                unique: false,
+                foreign_key: None,
+            }),
+        };
+
+        let tokens = op.repr();
+        let tokens_str = tokens.to_string();
+
+        assert!(
+            tokens_str.contains("alter_field"),
+            "Should call alter_field() but got: {tokens_str}"
+        );
+        assert!(
+            tokens_str.contains("table_name"),
+            "Should call table_name() but got: {tokens_str}"
+        );
+        assert!(
+            tokens_str.contains("old_field"),
+            "Should call old_field() but got: {tokens_str}"
+        );
+        assert!(
+            tokens_str.contains("new_field"),
+            "Should call new_field() but got: {tokens_str}"
+        );
+        assert!(
+            tokens_str.contains("build"),
+            "Should call build() but got: {tokens_str}"
+        );
+    }
 }

cot/src/db/migrations.rs

@@ -494,6 +494,17 @@ impl Operation {
                     .to_owned();
                 database.execute_schema(query).await?;
             }
+            OperationInner::AlterField {
+                table_name,
+                old_field: _,
+                new_field,
+            } => {
+                let query = sea_query::Table::alter()
+                    .table(*table_name)
+                    .modify_column(new_field.as_column_def(database))
+                    .to_owned();
+                database.execute_schema(query).await?;
+            }
             OperationInner::RemoveModel {
                 table_name,
                 fields: _,
@@ -560,6 +571,18 @@ impl Operation {
                     .to_owned();
                 database.execute_schema(query).await?;
             }
+            OperationInner::AlterField {
+                table_name,
+                old_field,
+                new_field: _,
+            } => {
+                // To reverse an alteration, set the column back to the old definition
+                let query = sea_query::Table::alter()
+                    .table(*table_name)
+                    .modify_column(old_field.as_column_def(database))
+                    .to_owned();
+                database.execute_schema(query).await?;
+            }
             OperationInner::RemoveModel { table_name, fields } => {
                 let mut query = sea_query::Table::create().table(*table_name).to_owned();
                 for field in *fields {
@@ -606,6 +629,11 @@ enum OperationInner {
         table_name: Identifier,
         fields: &'static [Field],
     },
+    AlterField {
+        table_name: Identifier,
+        old_field: Field,
+        new_field: Field,
+    },
 }
 
 /// A field in a model.
@@ -1530,6 +1558,50 @@ impl RemoveModelBuilder {
     }
 }
 
+pub const fn alter_field() -> AlterFieldBuilder {
+    AlterFieldBuilder::new()
+}
+
+#[derive(Debug, Copy, Clone)]
+pub struct AlterFieldBuilder {
+    table_name: Option<Identifier>,
+    old_field: Option<Field>,
+    new_field: Option<Field>,
+}
+
+impl AlterFieldBuilder {
+    const fn new() -> Self {
+        Self {
+            table_name: None,
+            old_field: None,
+            new_field: None,
+        }
+    }
+
+    pub const fn table_name(mut self, table_name: Identifier) -> Self {
+        self.table_name = Some(table_name);
+        self
+    }
+
+    pub const fn old_field(mut self, field: Field) -> Self {
+        self.old_field = Some(field);
+        self
+    }
+
+    pub const fn new_field(mut self, field: Field) -> Self {
+        self.new_field = Some(field);
+        self
+    }
+
+    pub const fn build(self) -> Operation {
+        Operation::new(OperationInner::AlterField {
+            table_name: unwrap_builder_option!(self, table_name),
+            old_field: unwrap_builder_option!(self, old_field),
+            new_field: unwrap_builder_option!(self, new_field),
+        })
+    }
+}
+
 /// A trait for defining a migration.
 ///
 /// # Cot CLI Usage