fix(storage): fix payment list sorting #298
Conversation
WalkthroughThe pull request modifies the Changes
Possibly related PRs
Suggested reviewers
Poem
✨ 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 (
|
paul-nicolas
force-pushed
the
fix/payments-list-sorting
branch
from
6cc2bf2 to
6412fdb
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #298 +/- ##
=======================================
Coverage 73.05% 73.06%
=======================================
Files 537 537
Lines 27115 27126 +11
=======================================
+ Hits 19810 19819 +9
- Misses 6238 6241 +3
+ Partials 1067 1066 -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: 0
🧹 Nitpick comments (1)
internal/storage/payments_test.go (1)
773-844: The test coverage looks good, consider adding more edge cases.The test effectively verifies the new sorting behavior with adjustments sharing the same timestamp. To make it more robust, consider adding test cases for:
- Multiple payments with adjustments at the same timestamp
- Explicit sort_id values to verify the ordering
- Edge cases like adjustments with millisecond differences
Here's a suggested additional test case:
func TestPaymentsListSorting(t *testing.T) { // ... existing test code ... + t.Run("multiple payments with same timestamp", func(t *testing.T) { + // Create two payments with adjustments at the same timestamp + // but different sort_ids to verify the ordering + }) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/storage/payments.go(1 hunks)internal/storage/payments_test.go(1 hunks)
🔇 Additional comments (1)
internal/storage/payments.go (1)
332-332: LGTM! The sorting logic enhancement looks good.The addition of
sort_idas a secondary sorting criterion ensures consistent ordering when multiple payment adjustments share the same timestamp. This effectively addresses the issue of distinguishing between pending and succeeded transactions.Let's verify that sort_id is being populated correctly in the payment_adjustments table:
✅ Verification successful
✅ Verified: sort_id is properly implemented and maintained
The
sort_idcolumn in thepayment_adjustmentstable is:
- Automatically populated (bigserial)
- Never null (constraint)
- Properly indexed for efficient sorting
The ORDER BY clause will work reliably as intended.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if sort_id column exists and contains valid values # Expected: Column exists and contains non-null values for recent adjustments # Check if sort_id column exists in payment_adjustments table psql -c "\d payment_adjustments" | grep -i "sort_id" # Check for any NULL sort_ids in recent adjustments psql -c "SELECT COUNT(*) FROM payment_adjustments WHERE sort_id IS NULL AND created_at > NOW() - INTERVAL '1 day'"Length of output: 365
Script:
#!/bin/bash # Search for sort_id in SQL statements and struct definitions echo "=== SQL Statements with sort_id ===" rg -i "sort_id" --type sql -A 3 -B 3 echo "=== Go struct definitions and usage ===" ast-grep --pattern 'type $_ struct { $$$ sort_id $_ $$$ }' echo "=== Migration files ===" fd -e sql migrationLength of output: 24637
Mor modulr, when we create a transfer/payout, we will have the same date for pending/succeeded transaction, so the only thing differenciating the two is the sort_id we added. That way, we can have the last status in the main payment