Fix MySQL JSON default and load engine migrations#33
Conversation
- Remove 'default: {}' from JSON metadata columns in both engine and app
migrations (MySQL doesn't support defaults on JSON/BLOB/TEXT columns)
- Add initializer to append engine migration paths so Rails picks them up
- Remove duplicate table creation from app migration — engine owns the
schema, app migration only handles data migration and FK rewiring
Amp-Thread-ID: https://ampcode.com/threads/T-019c9ba5-621e-7769-81b4-99b46ff27501
Co-authored-by: Amp <amp@ampcode.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d11067b4fa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| initializer "coplan.append_migrations", before: :load_config_initializers do |app| | ||
| config.paths["db/migrate"].expanded.each do |path| | ||
| app.config.paths["db/migrate"] << path | ||
| ActiveRecord::Migrator.migrations_paths << path |
There was a problem hiding this comment.
Appending engine/db/migrate at runtime makes Rails evaluate engine migration files in addition to any copied migrations from bin/rails coplan:install:migrations (as documented in docs/HOST_APP_GUIDE.md:19), which leads to duplicate migration name/version errors and blocks db:migrate in host apps that followed the documented install flow. This initializer should guard against double-registration (or choose one migration delivery mechanism) so migrations are not loaded twice.
Useful? React with 👍 / 👎.
| @@ -47,7 +37,5 @@ def down | |||
| add_foreign_key :coplan_plan_collaborators, :users, column: :user_id | |||
| add_foreign_key :coplan_plan_collaborators, :users, column: :added_by_user_id | |||
| add_foreign_key :coplan_plans, :users, column: :created_by_user_id | |||
There was a problem hiding this comment.
After this change, down no longer removes rows inserted into coplan_users, so a rollback/re-migrate cycle (db:rollback STEP=1 then db:migrate) will re-run the INSERT INTO coplan_users ... SELECT ... FROM users and hit duplicate primary-key/unique-key errors for existing users. This affects environments that test or rely on rollback/redeploy workflows.
Useful? React with 👍 / 👎.
1 participant
Problem
Migrations fail with:
And the engine's
coplan_usersmigration wasn't being picked up by the app, causing a second failure when the app migration tried to INSERT into a table that didn't exist yet.Changes
Remove
default: {}from JSON columns — MySQL doesn't support defaults on JSON/BLOB/TEXT columns. The model already handles this viaafter_initialize { self.metadata ||= {} }.Add engine migration path initializer — The engine wasn't appending its
db/migratepath, so Rails never discovered the engine's migrations. Added an initializer that appends the path early enough for the migrator to see it.Remove duplicate table creation from app migration — The engine migration (
20260226200000) owns thecoplan_userstable schema. The app migration (20260226200001) now only handles data migration fromusersand FK rewiring.