feat: [#358] Add Update Delete methods for DB #904
Conversation
WalkthroughThis pull request extends database query functionality by introducing Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Query
participant SQLBuilder
participant Database
Client->>Query: call Update(data)
Query->>SQLBuilder: buildUpdate(data)
SQLBuilder-->>Query: SQL statement & args
Query->>Database: execute SQL query
Database-->>Query: return result & error
Query-->>Client: return result & error
sequenceDiagram
participant Client
participant Query
participant SQLBuilder
participant Database
Client->>Query: call Delete()
Query->>SQLBuilder: buildDelete()
SQLBuilder-->>Query: SQL statement & args
Query->>Database: execute SQL query
Database-->>Query: return result & error
Query-->>Client: return result & error
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 golangci-lint (1.62.2)Error: can't load config: the Go language version (go1.23) used to build golangci-lint is lower than the targeted Go version (1.24.0) ✨ 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 (
|
| @@ -127,6 +127,9 @@ func convertToMap(data any) (map[string]any, error) { | |||
| if fieldValue.Kind() == reflect.Ptr && !fieldValue.IsNil() { | |||
| fieldValue = fieldValue.Elem() | |||
| } | |||
| if fieldValue.IsZero() { | |||
There was a problem hiding this comment.
Don't update or insert the zero value of struct.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #904 +/- ##
=======================================
Coverage 67.78% 67.78%
=======================================
Files 154 154
Lines 10194 10194
=======================================
Hits 6910 6910
Misses 2951 2951
Partials 333 333 ☔ 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: 1
🧹 Nitpick comments (2)
database/db/query.go (1)
147-174: Reduce code duplication in SQL building logic.The where clause handling logic is duplicated across
buildDelete,buildUpdate, andbuildSelect. Consider extracting this into a helper method.+func (r *Query) buildWhereClause(builder sq.SelectBuilder) sq.SelectBuilder { + for _, where := range r.conditions.where { + query, ok := where.query.(string) + if ok { + if !str.Of(query).Trim().Contains(" ", "?") { + if len(where.args) > 1 { + builder = builder.Where(sq.Eq{query: where.args}) + } else if len(where.args) == 1 { + builder = builder.Where(sq.Eq{query: where.args[0]}) + } + continue + } + } + builder = builder.Where(where.query, where.args...) + } + return builder +}Also applies to: 236-265
tests/db_test.go (1)
138-203: Consider adding error case tests.The test thoroughly validates the happy path for update and delete operations. Consider adding test cases for error scenarios such as:
- Updating non-existent records
- Deleting non-existent records
- Invalid data types in updates
📜 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 (8)
contracts/database/db/db.go(1 hunks)database/db/query.go(4 hunks)database/db/query_test.go(3 hunks)database/db/utils.go(1 hunks)database/db/utils_test.go(2 hunks)mocks/database/db/Query.go(2 hunks)tests/db_test.go(3 hunks)tests/go.mod(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/go.mod
🔇 Additional comments (10)
contracts/database/db/db.go (1)
11-11: LGTM! Well-structured interface additions.The new
Delete()andUpdate(data any)methods are consistent with the existing interface design and follow the same return type pattern asInsert().Also applies to: 14-14
database/db/utils_test.go (1)
10-10: LGTM! Comprehensive test coverage for the new field.The test cases thoroughly cover the new
Lengthfield across different scenarios including struct slices, pointer slices, and direct struct usage.Also applies to: 35-35, 45-45, 55-55, 57-57
database/db/utils.go (1)
130-132: LGTM! Zero-value handling implemented as requested.The implementation correctly skips zero-value fields during map conversion, addressing the previous feedback.
database/db/query.go (1)
31-52: LGTM! Well-implemented Delete and Update methods.The implementations handle errors appropriately, use proper SQL building, and return affected rows count.
Also applies to: 108-134
database/db/query_test.go (3)
16-21: LGTM!The
TestUserstruct has been updated with a newPhonefield and appropriate database tags.
38-48: LGTM!The test case thoroughly validates the deletion functionality by:
- Verifying the SQL statement construction
- Checking the rows affected
- Ensuring no errors occur
166-202: LGTM!The test case thoroughly validates the update functionality by:
- Testing both struct and map update scenarios
- Verifying SQL statement construction for each case
- Checking the rows affected
- Ensuring no errors occur
tests/db_test.go (1)
40-42: LGTM!Moving the
nowvariable initialization outside the loop improves performance by avoiding repeated initialization.mocks/database/db/Query.go (2)
23-78: LGTM!The mock implementation for
Deleteis well-structured and provides comprehensive mocking capabilities including:
- Proper error handling
- Type assertions
- Flexible Run/Return methods
230-286: LGTM!The mock implementation for
Updateis well-structured and provides comprehensive mocking capabilities including:
- Proper error handling
- Type assertions
- Flexible Run/Return methods
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/db_test.go (1)
138-203: LGTM! Well-structured test implementation.The test comprehensively covers update and delete operations with good validation of results.
Consider enhancing test coverage with edge cases.
Consider adding the following test cases to make the test suite more robust:
- Update/delete of non-existent records
- Verification of
updated_attimestamp changes- Update with invalid data types
Here's a suggested structure using sub-tests:
func (s *DBTestSuite) TestUpdate_Delete() { now := carbon.NewDateTime(carbon.FromDateTime(2025, 1, 2, 3, 4, 5)) for driver, query := range s.queries { s.Run(driver, func() { + s.Run("successful operations", func() { // Your existing test code + }) + + s.Run("edge cases", func() { + // Test updating non-existent record + result, err := query.DB().Table("products").Where("name", "non-existent").Update(map[string]any{ + "name": "updated", + }) + s.NoError(err) + s.Equal(int64(0), result.RowsAffected) + + // Test deleting non-existent record + result, err = query.DB().Table("products").Where("name", "non-existent").Delete() + s.NoError(err) + s.Equal(int64(0), result.RowsAffected) + + // Test update with invalid data type + result, err = query.DB().Table("products").Where("name", "test").Update(map[string]any{ + "created_at": "invalid-date", + }) + s.Error(err) + }) }) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/db_test.go(3 hunks)
⏰ Context from checks skipped due to timeout of 300000ms (1)
- GitHub Check: test / windows (1.23)
🔇 Additional comments (1)
tests/db_test.go (1)
41-41: LGTM! Good performance optimization.Moving the
nowvariable initialization outside the loop reduces unnecessary object creation.
📑 Description
goravel/goravel#358
Summary by CodeRabbit
New Features
Tests
Chores
✅ Checks