-
Notifications
You must be signed in to change notification settings - Fork 25
refactor: remove main mapping id from migration fix #83
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
refactor: remove main mapping id from migration fix #83
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the swag_migration_fix table to use entity_id directly instead of referencing the mapping table via main_mapping_id. This simplifies the data model by removing an unnecessary join.
- Removed
main_mapping_idfield fromSwagMigrationFixEntityandSwagMigrationFixDefinition - Updated
MigrationFixApplierSQL query to useentity_iddirectly instead of joining with the mapping table - Added database migration to drop the
main_mapping_idcolumn and foreign key - Updated all tests to use
entity_idinstead ofmain_mapping_id
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/Migration/MigrationFix/SwagMigrationFixEntity.php | Removed mainMappingId property and its getter/setter methods |
| src/Migration/MigrationFix/SwagMigrationFixDefinition.php | Removed main_mapping_id field definition from the entity schema |
| src/Migration/Writer/MigrationFix/MigrationFixApplier.php | Simplified SQL query to query entity_id directly instead of joining with mapping table |
| src/Core/Migration/Migration1762436233RemoveMainMappingIdFromMigrationFix.php | Added migration to drop main_mapping_id column and its foreign key constraint |
| tests/unit/Migration/Writer/MigrationFix/MigrationFixTest.php | Updated test data to use entity_id instead of main_mapping_id |
| tests/integration/Migration/Writer/MigrationFix/MigrationFixApplierTest.php | Removed unnecessary mapping creation and updated to use entity_id directly in createFix method |
| tests/integration/Migration/MigrationFix/SwagMigrationEntityTest.php | Updated to retrieve and use entity_uuid from mapping instead of mapping id |
| tests/TableHelperTrait.php | Added helper methods for managing foreign keys (foreignKeyExists, addForeignKey, dropForeignKey) |
| tests/Core/Migration/Migration1762436233RemoveMainMappingIdFromMigrationFixTest.php | Added test for the new migration that verifies column and foreign key removal |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/Core/Migration/Migration1762436233RemoveMainMappingIdFromMigrationFix.php
Show resolved
Hide resolved
…or' into refactor/remove-main-mapping-id-from-migration-fix
222c0bd
into
feature/migration-logging-refactor
resolves #13389
the main_mapping_id field is not needed anymore, so we can remove it