-
Notifications
You must be signed in to change notification settings - Fork 10
fix(Stripe): Reverse order of items returned by Stripe List APIs #505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThis update introduces helper functions to reverse the order of Stripe API results for accounts, bank accounts, and balance transactions, addressing Stripe's change to reverse chronological ordering. It also adds a logging field to the client, updates client instantiation to accept a logger, introduces a Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin
participant Logger
participant Client
participant Backend
participant StripeAPI
Plugin->>Logger: Pass logger to Client.New
Plugin->>Client: Instantiate client with logger, backend, apiKey
Client->>Backend: Make API call (e.g., GetPayments)
Backend->>StripeAPI: Perform HTTP request
StripeAPI-->>Backend: Return data (reverse chronological order)
Backend-->>Client: Return data
Client->>Client: Reverse data order (restore chronological)
Client->>Logger: Log debug info (e.g., latest ID, batch size)
Client-->>Plugin: Return processed results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (10)
🧰 Additional context used🧠 Learnings (6)📓 Common learnings📚 Learning: the `balance` struct in `internal/connectors/plugins/public/dummypay/client/client.go` is only used ...Applied to files:
📚 Learning: the qonto connector plugin has unit tests for fetchnextbalances in internal/connectors/plugins/publi...Applied to files:
📚 Learning: in `test/e2e/api_accounts_test.go`, the `subscribe` function already includes error handling, so it'...Applied to files:
📚 Learning: in the v2bankaccountmessagepayload struct, the relatedaccounts field is intentionally mapped to json...Applied to files:
📚 Learning: the debug flag in the payments service is primarily used to control the visibility of dummypay in co...Applied to files:
🧬 Code Graph Analysis (1)internal/connectors/plugins/public/stripe/plugin.go (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (17)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #505 +/- ##
==========================================
- Coverage 68.75% 68.70% -0.06%
==========================================
Files 715 715
Lines 36816 36846 +30
==========================================
+ Hits 25313 25315 +2
- Misses 10182 10209 +27
- Partials 1321 1322 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question -- how did we detect this change?
Can we / should we have some contract testing?
|
COntract testing as in more like "monitoring testing" -- run a set of queries, make sure the response don't change. |
I was testing something unrelated and realized that it was wonky 🙁
I guess we'd have to set up some tests that run against the actual PSP using hardcoded tokens? We could maybe do that in a private repo to avoid leaking tokens. If you have some specific ideas might be worth exploring. |
Yeah that's what I mean. |
Fixes: PMNT-136
Hard to know when this changed, because there are also mentions of
ending_beforereversing the order of transactions returned, which was the case when we implemented this API. Now the docs say that the order of transactions are always returned in "reverse chronological order":https://docs.stripe.com/api/pagination
Regarding the Refund fix:
Two adjustments indicating an initial amount of 116.26 and an adjustment of -35.00
And the resulting payment: