feat: [#571] migrator supports After and First column modifiers #876
Conversation
WalkthroughThis pull request modifies the schema migration interfaces and implementations to enhance column ordering capabilities and clarify database compatibility in method documentation. The Changes
Sequence Diagram(s)sequenceDiagram
participant U as User/Migration Script
participant B as Blueprint
participant C as ColumnDefinition
participant DB as Database Schema
U->>B: Define new column with ordering details
B->>C: Call After/First methods to set column position
C->>DB: Update ordering parameters (fields: after, first)
DB-->>C: Confirm update
C-->>B: Return updated ColumnDefinition
Assessment against linked issues
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #876 +/- ##
=======================================
Coverage 67.51% 67.51%
=======================================
Files 151 151
Lines 10355 10355
=======================================
Hits 6991 6991
Misses 2996 2996
Partials 368 368 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
contracts/database/schema/column.go (1)
16-17: Consider adding database compatibility comments to getter methods.The getter methods
GetAfterandIsFirstshould follow the same pattern as other methods and include comments indicating MySQL-only support for consistency.Also applies to: 50-51
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
contracts/database/schema/blueprint.go(1 hunks)contracts/database/schema/column.go(3 hunks)database/schema/column.go(4 hunks)mocks/database/schema/ColumnDefinition.go(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- contracts/database/schema/blueprint.go
🔇 Additional comments (6)
contracts/database/schema/column.go (2)
4-5: LGTM! Clear documentation of MySQL-only support.The
Aftermethod is well-documented with a clear comment indicating MySQL-only support.
14-15: LGTM! Clear documentation of MySQL-only support.The
Firstmethod is well-documented with a clear comment indicating MySQL-only support.database/schema/column.go (3)
9-9: LGTM! Appropriate field types.The
afterandfirstfields are appropriately typed asstringandboolrespectively.Also applies to: 15-15
36-40: LGTM! Clean implementation of column positioning methods.The
AfterandFirstmethods have a clean implementation that follows the builder pattern by returningself.Also applies to: 66-70
72-74: LGTM! Simple getter implementations.The getter methods
GetAfterandIsFirsthave straightforward implementations that directly return their respective field values.Also applies to: 188-190
mocks/database/schema/ColumnDefinition.go (1)
23-69: LGTM! Consistent mock implementations.The auto-generated mock code follows the established patterns and includes proper error handling and type assertions.
Also applies to: 261-306, 308-351, 1079-1122
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
database/schema/column.go (1)
36-40: Consider adding column name validation.While the implementation is correct, consider validating the column name to prevent SQL injection or invalid column names.
func (r *ColumnDefinition) After(column string) schema.ColumnDefinition { + // Validate column name to prevent SQL injection + if !isValidColumnName(column) { + panic("invalid column name") + } r.after = column return r }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
contracts/database/schema/blueprint.go(1 hunks)contracts/database/schema/column.go(3 hunks)database/schema/column.go(4 hunks)mocks/database/schema/ColumnDefinition.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- contracts/database/schema/column.go
- contracts/database/schema/blueprint.go
⏰ Context from checks skipped due to timeout of 300000ms (2)
- GitHub Check: test / windows (1.23)
- GitHub Check: test / windows (1.22)
🔇 Additional comments (4)
database/schema/column.go (3)
9-15: LGTM! Field additions are well-structured.The new fields
afterandfirstare appropriately typed and follow the existing field ordering pattern.
66-70: LGTM! First method implementation is clean and consistent.The method follows the builder pattern and correctly implements method chaining.
72-74: LGTM! Getter methods are properly implemented.Both
GetAfterandIsFirstmethods follow the established getter pattern in the codebase.Also applies to: 188-190
mocks/database/schema/ColumnDefinition.go (1)
1-1514: LGTM! Auto-generated mock implementation is complete.The mock implementation correctly includes all new methods and follows the established mockery patterns.
hwbrzzl
left a comment
hwbrzzl
left a comment
There was a problem hiding this comment.
LGTM, please remember to add tests in tests.
| // AutoIncrement set the column as auto increment | ||
| AutoIncrement() ColumnDefinition | ||
| // Change the column | ||
| // Change the column (MySQL / PostgreSQL / SQL Server) |
Okay, I will submit another PR. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
database/schema/column.go (3)
9-9: Initialize new fields in the constructor.The new fields
afterandfirstshould be initialized with zero values in theNewColumnDefinitionconstructor to ensure consistent behavior.func NewColumnDefinition(name string, ttype string) schema.ColumnDefinition { return &ColumnDefinition{ name: &name, ttype: convert.Pointer(ttype), + after: "", + first: false, } }Also applies to: 15-15
36-40: Add validation for the After method.Consider adding validation to ensure the column name is not empty and follows database naming conventions.
func (r *ColumnDefinition) After(column string) schema.ColumnDefinition { + if column == "" { + // Either return an error or log a warning + return r + } r.after = column return r }
8-27: Add documentation about database compatibility.Since these column modifiers are specifically for MySQL (as mentioned in the PR objectives), consider adding documentation to clarify the database compatibility of these features.
type ColumnDefinition struct { + // after specifies the column after which this column should be placed (MySQL only) after string allowed []any autoIncrement *bool change bool comment *string def any + // first indicates if this column should be placed first in the table (MySQL only) first bool length *int name *string
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
contracts/database/schema/blueprint.go(1 hunks)contracts/database/schema/column.go(3 hunks)database/schema/column.go(4 hunks)mocks/database/schema/ColumnDefinition.go(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- contracts/database/schema/blueprint.go
- contracts/database/schema/column.go
- mocks/database/schema/ColumnDefinition.go
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: test / windows (1.22)
🔇 Additional comments (1)
database/schema/column.go (1)
66-70: LGTM!The implementations of
First,GetAfter, andIsFirstmethods are clean, follow the established patterns, and correctly implement the required functionality.Also applies to: 72-74, 188-190
📑 Description
Go migrator supports
AfterandFirstcolumn modifiers. (MySQL only)Closes goravel/goravel#571
Summary by CodeRabbit
New Features
Documentation
✅ Checks