-
Notifications
You must be signed in to change notification settings - Fork 105
chore: merge v1.15.3 #842
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
chore: merge v1.15.3 #842
Conversation
WalkthroughThe pull request makes several changes across multiple components. It improves test reliability by using a single timestamp for time-related assertions, extends the database schema functionality by adding new methods ( Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Blueprint
participant createAndAddColumn
Caller->>Blueprint: Call Boolean(column)
Blueprint->>createAndAddColumn: Create boolean column
createAndAddColumn-->>Blueprint: Return ColumnDefinition
Blueprint-->>Caller: Provide ColumnDefinition
sequenceDiagram
participant Caller
participant Grammar
Caller->>Grammar: Call TypeBoolean(ColumnDefinition)
Grammar-->>Caller: Return type string for boolean column
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 #842 +/- ##
==========================================
+ Coverage 67.12% 67.16% +0.03%
==========================================
Files 150 150
Lines 10442 10442
==========================================
+ Hits 7009 7013 +4
+ Misses 3058 3055 -3
+ Partials 375 374 -1 ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
database/schema/blueprint.go (1)
85-87: Consider adding type validation.The Column method provides flexibility for creating columns of any type, but it might benefit from validation to ensure only supported types are used.
Consider adding type validation:
func (r *Blueprint) Column(column, ttype string) schema.ColumnDefinition { + validTypes := map[string]bool{ + "bigInteger": true, "boolean": true, "char": true, + "date": true, "dateTime": true, "decimal": true, + "double": true, "enum": true, "float": true, + "integer": true, "json": true, "jsonb": true, + "longText": true, "mediumInteger": true, "mediumText": true, + "smallInteger": true, "string": true, "text": true, + "time": true, "timestamp": true, "tinyInteger": true, + "tinyText": true, + } + if !validTypes[ttype] { + panic(fmt.Sprintf("Unsupported column type: %s", ttype)) + } return r.createAndAddColumn(ttype, column) }database/schema/blueprint_test.go (1)
110-117: Consider adding more test cases.While the basic test case is good, consider adding tests for:
- Column modifiers (e.g., nullable, default value)
- Column changes using Change()
- Error cases
Example additional test cases:
func (s *BlueprintTestSuite) TestBoolean() { name := "name" s.blueprint.Boolean(name) s.Contains(s.blueprint.GetAddedColumns(), &ColumnDefinition{ name: &name, ttype: convert.Pointer("boolean"), }) + + // Test with nullable modifier + s.blueprint.Boolean(name).Nullable() + s.Contains(s.blueprint.GetAddedColumns(), &ColumnDefinition{ + name: &name, + ttype: convert.Pointer("boolean"), + nullable: convert.Pointer(true), + }) + + // Test with default value + s.blueprint.Boolean(name).Default(true) + s.Contains(s.blueprint.GetAddedColumns(), &ColumnDefinition{ + name: &name, + ttype: convert.Pointer("boolean"), + def: convert.Pointer(true), + }) + + // Test with Change modifier + s.blueprint.Boolean(name).Change() + s.Contains(s.blueprint.GetAddedColumns(), &ColumnDefinition{ + name: &name, + ttype: convert.Pointer("boolean"), + change: true, + }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/go.sumis excluded by!**/*.sum
📒 Files selected for processing (13)
auth/auth_test.go(2 hunks)contracts/database/schema/blueprint.go(1 hunks)contracts/database/schema/grammar.go(1 hunks)database/gorm/logger.go(0 hunks)database/schema/blueprint.go(2 hunks)database/schema/blueprint_test.go(1 hunks)database/schema/utils.go(1 hunks)database/schema/utils_test.go(1 hunks)mocks/database/schema/Blueprint.go(2 hunks)mocks/database/schema/Grammar.go(1 hunks)support/constant.go(1 hunks)tests/go.mod(2 hunks)tests/schema_test.go(11 hunks)
💤 Files with no reviewable changes (1)
- database/gorm/logger.go
✅ Files skipped from review due to trivial changes (3)
- auth/auth_test.go
- support/constant.go
- tests/go.mod
🧰 Additional context used
🪛 golangci-lint (1.62.2)
tests/schema_test.go
175-175: table.Boolean undefined (type "github.com/goravel/framework/contracts/database/schema".Blueprint has no field or method Boolean)
(typecheck)
2506-2506: table.Boolean undefined (type "github.com/goravel/framework/contracts/database/schema".Blueprint has no field or method Boolean)
(typecheck)
2509-2509: table.Column undefined (type "github.com/goravel/framework/contracts/database/schema".Blueprint has no field or method Column)
(typecheck)
2511-2511: table.Column undefined (type "github.com/goravel/framework/contracts/database/schema".Blueprint has no field or method Column)
(typecheck)
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: test / windows (1.23)
🔇 Additional comments (13)
contracts/database/schema/grammar.go (1)
66-67: LGTM! Well-structured interface addition.The new
TypeBooleanmethod follows the established pattern of other type methods in the interface, maintaining consistency in the API design.contracts/database/schema/blueprint.go (2)
12-13: LGTM! Well-structured Boolean method addition.The new
Booleanmethod follows the established pattern of other column definition methods in the interface.
18-19: LGTM! Well-structured Column method addition.The new
Columnmethod provides flexibility for custom column types while maintaining consistency with the interface design.database/schema/utils.go (1)
42-42: LGTM! Improved error handling.The change to return
column.GetType()instead of an empty string preserves valuable type information when a method is not found, making it easier to debug issues with invalid column types.database/schema/utils_test.go (1)
40-40: LGTM! Test updates match implementation changes.The test changes correctly reflect the new behavior where:
GetType()is called twice due to the implementation change- The assertion now expects the actual type ("invalid") instead of an empty string
Also applies to: 44-44
database/schema/blueprint.go (1)
59-61: LGTM! The Boolean method follows the established pattern.The implementation correctly uses the createAndAddColumn function and follows the same pattern as other type methods in the file.
mocks/database/schema/Grammar.go (1)
1501-1545: LGTM! The mock implementation follows the established pattern.The TypeBoolean mock method is correctly implemented with all necessary mock functionality, following the same pattern as other type methods in the file.
tests/schema_test.go (3)
175-175: LGTM! Boolean column type implementation is consistent across databases.The boolean column type implementation is correctly tested across different databases with appropriate type mappings:
- Postgres: boolean/bool
- SQLite: tinyint(1)
- MySQL: tinyint(1)
- SQL Server: bit
The default value handling is also consistent:
- Postgres: 'true'
- SQLite: '1'
- MySQL: '1'
- SQL Server: '('1')'
Also applies to: 289-297, 628-635, 928-936, 1260-1268
🧰 Tools
🪛 golangci-lint (1.62.2)
175-175: table.Boolean undefined (type "github.com/goravel/framework/contracts/database/schema".Blueprint has no field or method Boolean)
(typecheck)
2508-2512: LGTM! Custom type column implementation handles database-specific types correctly.The implementation correctly handles database-specific custom types:
- Postgres: Uses 'macaddr' type
- Other databases: Uses 'geometry' type
The conditional type selection ensures compatibility across different databases.
Also applies to: 316-323, 652-659, 955-962, 1287-1294
🧰 Tools
🪛 golangci-lint (1.62.2)
2509-2509: table.Column undefined (type "github.com/goravel/framework/contracts/database/schema".Blueprint has no field or method Column)
(typecheck)
2511-2511: table.Column undefined (type "github.com/goravel/framework/contracts/database/schema".Blueprint has no field or method Column)
(typecheck)
2552-2552: LGTM! Column count updated to include new columns.The test correctly verifies the expected number of columns (35) after adding boolean_default and custom_type columns.
mocks/database/schema/Blueprint.go (3)
120-166: LGTM! Boolean method mock implementation follows established patterns.The Boolean method mock is correctly implemented with:
- Proper error handling for unspecified return values
- Type-safe method shadowing
- Helper methods for defining expectations
278-325: LGTM! Column method mock implementation follows established patterns.The Column method mock is correctly implemented with:
- Proper error handling for unspecified return values
- Type-safe method shadowing
- Helper methods for defining expectations
Line range hint
1-1: Verify Blueprint interface compatibility.The static analysis hints indicate that the Blueprint interface may be missing the Boolean and Column method declarations.
Run the following script to verify the interface declarations:
✅ Verification successful
Blueprint interface compatibility verified.
The shell output confirms that both the
BooleanandColumnmethods exist as implemented on the mock type, ensuring compatibility with the Blueprint interface.
- The
Booleanmethod is implemented (seefunc (_m *Blueprint) Boolean(...and its expecter counterpart).- The
Columnmethod is implemented (seefunc (_m *Blueprint) Column(...along with its expecter version).🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if Blueprint interface declares the Boolean and Column methods # Search for Blueprint interface declaration and its methods ast-grep --pattern $'interface Blueprint { $$$ Boolean(column string) ColumnDefinition $$$ }' ast-grep --pattern $'interface Blueprint { $$$ Column(column string, ttype string) ColumnDefinition $$$ }'Length of output: 214
Script:
#!/bin/bash echo "Extracting the Blueprint interface block from mocks/database/schema/Blueprint.go ..." sed -n '/interface Blueprint {/,/}/p' mocks/database/schema/Blueprint.go echo "-----------------------" echo "Searching for 'Boolean' method declarations in mocks/database/schema/Blueprint.go ..." rg "Boolean\s*\(" mocks/database/schema/Blueprint.go echo "-----------------------" echo "Searching for 'Column' method declarations in mocks/database/schema/Blueprint.go ..." rg "Column\s*\(" mocks/database/schema/Blueprint.goLength of output: 1327
📑 Description
Summary by CodeRabbit
New Features
Behavior Improvements
Chores
✅ Checks