fix: prevent overwriting existing migrations when registering new ones#1141
fix: prevent overwriting existing migrations when registering new ones#1141hwbrzzl merged 4 commits intogoravel:masterfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug in the schema migration registration system where calling Register multiple times would overwrite previously registered migrations instead of accumulating them. The fix changes the behavior to append new migrations to the existing list rather than replacing it entirely.
Key changes:
- Modified the
Registermethod to append migrations instead of overwriting them - Added nil check initialization for the migrations slice
database/schema/schema.go
Outdated
| r.migrations = make([]contractsschema.Migration, 0) | ||
| } | ||
|
|
||
| r.migrations = append(r.migrations, migrations...) |
There was a problem hiding this comment.
This implementation may allow duplicate migrations to be registered if the same migration is passed to Register multiple times. Consider checking for duplicates before appending to prevent potential issues with migration execution.
database/schema/schema.go
Outdated
| if r.migrations == nil { | ||
| r.migrations = make([]contractsschema.Migration, 0) | ||
| } | ||
|
|
There was a problem hiding this comment.
[nitpick] The nil check should ideally be handled during schema initialization rather than in every Register call. This adds unnecessary overhead when Register is called multiple times.
| if r.migrations == nil { | |
| r.migrations = make([]contractsschema.Migration, 0) | |
| } |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1141 +/- ##
=======================================
Coverage 68.44% 68.44%
=======================================
Files 221 221
Lines 14319 14319
=======================================
Hits 9800 9800
Misses 4144 4144
Partials 375 375 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
database/schema/schema.go
Outdated
| if migrations == nil { | ||
| migrations = make([]contractsschema.Migration, 0) | ||
| } |
There was a problem hiding this comment.
This judgment is unnecessary.
|
Hey @ahmed3mar I left a message in Discord, please take a look. |
|
Hey @ahmed3mar I sent a private message in Discord, want to confirm your country and send a v1.16 commemorative T-shirt to you. Please contact me. |
📑 Description
Closes https://github.com/goravel/goravel/issues/
When uses Register several times to overrides the registered ones!
✅ Checks