fix: [#280] prefix is not added to table #665
Conversation
WalkthroughThe changes in this pull request involve significant modifications to the Changes
Possibly related PRs
Suggested reviewers
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 #665 +/- ##
==========================================
- Coverage 70.58% 70.57% -0.01%
==========================================
Files 186 186
Lines 11874 11877 +3
==========================================
+ Hits 8381 8382 +1
- Misses 2923 2924 +1
- Partials 570 571 +1 ☔ 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: 4
🧹 Outside diff range and nitpick comments (8)
database/gorm/query.go (1)
53-56: Clarify error message for missing database configurationThe error message
"not found database configuration"could be more descriptive to aid troubleshooting.Consider updating the error message to specify which connection is missing configuration:
if len(writeConfigs) == 0 { - return nil, errors.New("not found database configuration") + return nil, errors.New("no database configuration found for connection: " + connection) }database/gorm/query_test.go (7)
Line range hint
2709-2737: Handle potential errors when assertingNotNilIn
TestCustomConnection, after callingquery.Create(&person), you assert that an error should occur and thatperson.IDshould be0:assert.NotNil(t, query.Create(&person)) assert.True(t, person.ID == 0)Ensure that the error returned by
Createis the expected one, and consider usingassert.Errorwith a specific error message to make the test more robust.
Line range hint
2325-2339: Avoid direct comparison withnilin error assertionsIn
TestObserverEvent, you're comparing errors directly withnil:assert.EqualError(t, getObserverEvent(contractsorm.EventRetrieved, &UserObserver{})(nil), "retrieved")Consider using
assert.Errororassert.NoErrorfor better clarity and consistency in your tests.
Line range hint
2429-2443: Synchronize goroutines inTestLockForUpdateto prevent data racesIn
TestLockForUpdate, you're launching goroutines without synchronization, which may lead to data races:for i := 0; i < 10; i++ { go func() { // ... }() }Use a
sync.WaitGroupto wait for all goroutines to finish before proceeding. Also, ensure that shared resources are properly synchronized.
Line range hint
2761-2771: Check for errors after rolling back transactionsIn
TestTransactionError, after rolling back the transaction, you proceed without checking for errors:tx, err := query.Query().Begin() s.Nil(err) s.Nil(tx.Create(&user)) // ... s.Nil(tx.Rollback())Ensure you check the error returned by
Rollbackto handle any potential issues during the rollback process.
Line range hint
2874-2880: Handle errors returned bySavein testsIn
TestEvent_IsDirty, when saving the user after modification, you should handle the error returned bySave:s.Nil(query.Query().Save(&user))Consider capturing and asserting the error to ensure that the save operation is successful.
Line range hint
3196-3202: Remove redundant assertions afterCreateoperationsIn
TestPaginate, you assert thatuser3.ID > 0after creation:user3 := User{Name: "paginate_user", Avatar: "paginate_avatar3"} s.Nil(query.Query().Create(&user3)) s.True(user3.ID > 0)This assertion is redundant since
Createshould already ensure that the ID is set. You can remove this assertion to keep the test concise.
Line range hint
3388-3390: Correct duplicate test names inTestFirstOrFailIn
TestFirstOrFail, it appears there are duplicate test cases with the name "trigger when FirstOrFail". This could cause confusion when interpreting test results.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- database/gorm/event.go (1 hunks)
- database/gorm/query.go (6 hunks)
- database/gorm/query_test.go (1 hunks)
- foundation/application_test.go (1 hunks)
🔇 Additional comments (5)
database/gorm/query.go (5)
35-39: Well-structured incorporation offullConfigintoNewQueryThe addition of
fullConfigto theNewQueryfunction and theQuerystruct enhances the configuration management and allows for more comprehensive handling of database settings.
63-63: Ensure correct selection of write configurationWhen accessing
writeConfigs[0], ensure that the write configurations are ordered correctly, and the first one is the intended default.Please verify that
writeConfigs[0]is the appropriate configuration to use as the default. If there are multiple write configurations, confirm that the selection logic aligns with the expected behavior.
Line range hint
1125-1129: Consistent use offullConfigin nested queriesThe use of
r.fullConfigwhen creating a newQueryinstance within thebuildWithmethod ensures consistent configuration across nested queries.
1259-1260: Maintain consistency innewmethodThe
newmethod correctly passesr.fullConfigto maintain consistent configuration in newQueryinstances.
Line range hint
1319-1337: Verify connection comparison logic inrefreshConnectionEnsure that the comparison
connection == r.fullConfig.Connectionaccurately determines when to refresh the connection, considering any potential differences in connection naming conventions or case sensitivity.Please confirm that
connectionandr.fullConfig.Connectionare consistently formatted and that there are no edge cases where the comparison might fail due to casing or whitespace differences.
📑 Description
If the DB configuration sets the prefix parameter:
goravel_, then callfacades.Orm().Query().Table("users").Get(), the SQL will beselect * from users, expect:select * from goravel_users.Due to the Orm module having a huge refactor, this change will not be merged to v1.14.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✅ Checks