fix(sqlbuilder): fixed WithWhere/WithOrderBy for empty builder #39
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #39 +/- ##
==========================================
+ Coverage 75.28% 75.30% +0.01%
==========================================
Files 42 42
Lines 1748 1749 +1
==========================================
+ Hits 1316 1317 +1
Misses 318 318
Partials 114 114
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Here's the code health analysis summary for commits Analysis Summary
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @cnlangzi - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 2 issues found
- 🟢 Complexity: all looks good
- 🟢 Docstrings: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| @@ -207,6 +207,26 @@ func TestBuilder(t *testing.T) { | |||
| require.Equal(t, now, vars[2]) | |||
| }, | |||
| }, | |||
| { | |||
| name: "build_with_empty_where", | |||
There was a problem hiding this comment.
suggestion (testing): Test case for empty WHERE clause should verify SQL syntax correctness.
The test verifies no errors and correct variable count, but it should also ensure that the SQL statement does not contain a WHERE clause when conditions are not added.
| name: "build_with_empty_where", | |
| name: "build_with_empty_where", | |
| build: func() *Builder { | |
| b := New().Select("orders") | |
| return b | |
| }, | |
| wantQuery: "SELECT * FROM orders", | |
| wantArgs: []interface{}{}, | |
| wantErr: false, |
| @@ -78,6 +78,22 @@ func TestOrderByBuilder(t *testing.T) { | |||
| }, | |||
| wanted: "SELECT * FROM users ORDER BY id ASC, created_at DESC, updated_at ASC", | |||
| }, | |||
| { | |||
| name: "with_empty_order_by_should_work", | |||
There was a problem hiding this comment.
suggestion (testing): Test case for empty ORDER BY should verify SQL syntax correctness.
The test checks for the absence of ORDER BY in the SQL, but it should also ensure that no ORDER BY clause is appended when no valid columns are specified.
| name: "with_empty_order_by_should_work", | |
| { | |
| name: "with_empty_order_by_should_work", | |
| build: func() *Builder { | |
| b := New("SELECT * FROM users") | |
| b.OrderBy() // No columns specified | |
| return b | |
| }, | |
| wanted: "SELECT * FROM users", | |
| }, |
Fixes
WithWhereandWithOrderBy