Conversation
WalkthroughThis pull request enhances the database query interface by introducing a new Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant Q as Query Instance
participant WB as Where Builder
C->>Q: Call OrWhere(query, args...)
Q->>WB: buildWhere(new condition with or=true)
WB-->>Q: Return SQL condition
Q->>WB: buildWheres(updated conditions)
WB-->>Q: Return consolidated WHERE clause
Q-->>C: Return new Query instance with OR condition applied
sequenceDiagram
participant LMC as ListenerMakeCommand
participant F as FileUtility
participant CC as ConsoleContext
LMC->>F: file.PutContent(...)
alt Error occurs
F-->>LMC: Return error
LMC->>CC: Log error using ctx.Error(err.Error())
LMC-->>CC: Complete without propagating error
else No error
F-->>LMC: Success
LMC-->>CC: Complete normally
end
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 (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #910 +/- ##
==========================================
+ Coverage 68.01% 68.82% +0.81%
==========================================
Files 154 154
Lines 10192 10191 -1
==========================================
+ Hits 6932 7014 +82
+ Misses 2934 2855 -79
+ Partials 326 322 -4 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
database/db/query_test.go (1)
406-419: Added commented-out test case.There's a commented-out test case for "nested condition" that's not being used. Consider either implementing it or removing it completely to avoid confusion.
-// s.Run("nested condition", func() { -// var users []TestUser - -// s.mockDriver.EXPECT().Config().Return(database.Config{}).Once() -// s.mockBuilder.EXPECT().Select(&users, "SELECT * FROM users WHERE age IN (?,?) AND name = ?", 25, 30, "John").Return(nil).Once() -// s.mockDriver.EXPECT().Explain("SELECT * FROM users WHERE age IN (?,?) AND name = ?", 25, 30, "John").Return("SELECT * FROM users WHERE age IN (25,30) AND name = \"John\"").Once() -// s.mockLogger.EXPECT().Trace(s.ctx, s.now, "SELECT * FROM users WHERE age IN (25,30) AND name = \"John\"", int64(0), nil).Return().Once() - -// err := s.query.Where(func(query db.Query) { -// query.Where("age", []int{25, 30}).Where("name", "John") -// }).Get(&users) -// s.Nil(err) -// })database/db/query.go (1)
303-346: Comprehensive buildWheres implementation.The buildWheres method effectively combines multiple WHERE conditions with proper handling of OR conditions. The logic for building nested AND/OR conditions is well-implemented and covers all scenarios.
One minor suggestion: consider adding comments to explain the logic flow for future maintainers.
func (r *Query) buildWheres(wheres []Where) (sq.Sqlizer, error) { if len(wheres) == 0 { return nil, nil } var sqlizers []sq.Sqlizer for _, where := range wheres { query, args := r.buildWhere(where) sqlizer, err := r.toSqlizer(query, args) if err != nil { return nil, err } if where.or && len(sqlizers) > 0 { + // Process OR condition by wrapping previous conditions // If it's an OR condition and we have previous conditions, // wrap the previous conditions in an AND and create an OR condition if len(sqlizers) == 1 { + // Single previous condition - create simple OR sqlizers = []sq.Sqlizer{ sq.Or{ sqlizers[0], sqlizer, }, } } else { + // Multiple previous conditions - AND them together, then OR with new condition sqlizers = []sq.Sqlizer{ sq.Or{ sq.And(sqlizers), sqlizer, }, } } } else { // For regular WHERE conditions or the first condition sqlizers = append(sqlizers, sqlizer) } } + // Return single condition or AND multiple conditions together if len(sqlizers) == 1 { return sqlizers[0], nil } return sq.And(sqlizers), nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
contracts/database/db/db.go(1 hunks)database/db/conditions.go(1 hunks)database/db/query.go(6 hunks)database/db/query_test.go(10 hunks)event/console/listener_make_command.go(1 hunks)mocks/database/db/Query.go(1 hunks)tests/db_test.go(10 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
event/console/listener_make_command.go
[warning] 50-51: event/console/listener_make_command.go#L50-L51
Added lines #L50 - L51 were not covered by tests
🔇 Additional comments (18)
database/db/conditions.go (1)
11-11: Expanding query functionality with OR conditions ✅The addition of the
orfield to theWherestruct is a good enhancement that enables logical OR operations in query conditions. This field will determine whether a condition should be joined using OR logic instead of the default AND logic.contracts/database/db/db.go (1)
53-53: Adding OrWhere method enriches the Query interface ✅The addition of the
OrWheremethod to theQueryinterface allows for more flexible query building with OR conditions. This implementation follows the same pattern as the existingWheremethod, maintaining consistency in the API design.tests/db_test.go (3)
16-16: Centralized timestamp management improves test consistency ✅Adding the
nowfield to theDBTestSuitestruct centralizes timestamp management, which improves test consistency and maintainability by reducing duplication.
28-28: Good initialization of the centralized timestamp ✅The timestamp is properly initialized in the
SetupSuitemethod, ensuring it's available for all test cases.
242-264: Well-structured test case for OrWhere functionality ✅The
TestOrWherefunction effectively validates the newOrWheremethod by testing its ability to retrieve records that match either of the specified conditions. The test structure follows the existing patterns in the test suite, maintaining consistency.database/db/query_test.go (5)
58-61: Test expectations updated to use parenthesized WHERE clauses.The SQL queries in the mock expectations now include parentheses around the WHERE clause conditions, which aligns with the implementation of the new query builder.
72-74: Consistent formatting of WHERE clauses in error test cases.The test cases for error handling have been consistently updated to match the new parenthesized WHERE clause format.
85-87: Mock expectations consistently updated for final test case.The row affected error test case is also consistently updated to use the parenthesized WHERE format.
374-376: Test case naming improved for clarity.The test case names have been updated to be more descriptive, making the purpose of each test clearer.
421-436: New test method for OrWhere functionality.The new
TestOrWheremethod properly tests the ability to combine conditions with OR operators. The test demonstrates chaining multiple conditions with both Where and OrWhere methods.database/db/query.go (6)
151-161: Well-implemented OrWhere method.The OrWhere method correctly creates a new Query, maintains existing conditions, and adds the new condition with the 'or' flag set to true. This implementation enables proper chaining of query conditions.
214-217: Improved error handling in buildDelete.The method now explicitly returns errors from buildWheres, ensuring that any issues with WHERE clause construction are properly propagated.
262-267: Consistent error handling in buildSelect.Similar to buildDelete, this method now properly handles and returns errors from the buildWheres function.
280-286: Simplified buildUpdate method.The buildUpdate method now uses the common buildWheres function and directly returns the result of the builder's ToSql method, making the code more consistent and maintainable.
288-301: Well-designed buildWhere helper method.This method correctly handles different types of where conditions, including string-based conditions with special handling for single and multiple arguments, improving code reusability.
360-371: Type-safe toSqlizer helper method.The toSqlizer method properly handles different input types and provides clear error messages for unsupported types, which will aid in debugging and development.
mocks/database/db/Query.go (2)
230-251: Mock implementation for OrWhere method.The OrWhere method mock correctly handles the query parameter and variadic args, combining them into a single slice for the mock.Called method. The return value handling is also properly implemented.
253-287: Complete support for mocking OrWhere method.The implementation includes all necessary helper types and methods for setting up mock expectations, running custom behavior, and returning values, which matches the pattern used for other methods in the file.
There was a problem hiding this comment.
PR Overview
This PR introduces flexible query filtering by adding an OrWhere method for both production and mock queries while also updating related tests and documentation. Key changes include the new OrWhere implementation in both the production and test code, updated grouping of WHERE conditions in query builders, and adjusted error handling in the listener make command.
Reviewed Changes
| File | Description |
|---|---|
| mocks/database/db/Query.go | Adds mock functions and helpers for the new OrWhere method. |
| database/db/query.go | Implements the OrWhere method and updates query builders to include grouped conditions. |
| database/db/query_test.go | Updates expected query strings to reflect the new grouping with OrWhere. |
| tests/db_test.go | Adjusts time handling and adds a test for OrWhere usage. |
| database/db/conditions.go | Minor formatting update in the Conditions struct. |
| event/console/listener_make_command.go | Changes error handling: logs error on context instead of returning the error. |
| contracts/database/db/db.go | Updates interface comments to include the OrWhere method. |
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
📑 Description
goravel/goravel#358
Summary by CodeRabbit
New Features
Refactor
✅ Checks