-
Notifications
You must be signed in to change notification settings - Fork 393
Backport: API credit top-up telemetry + topupTrackerStore (#6500) #6502
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
…udit events\n\n- Add TelemetryEvents.API_CREDIT_TOPUP_SUCCEEDED and provider method\n- Emit on credit_added events in useCustomerEventsService with in-memory dedupe\n- No local storage, naive based on backend audit log (cherry picked from commit 0a50e43)
(cherry picked from commit 37d8a53)
(cherry picked from commit fbf2aeb)
🎭 Playwright Test Results❌ Some tests failed ⏰ Completed at: 11/01/2025, 05:25:14 AM UTC 📈 Summary
📊 Test Reports by Browser
🎉 Click on the links above to view detailed test results for each browser configuration. |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const reconcileByFetchingEvents = async (): Promise<boolean> => { | ||
| const service = useCustomerEventsService() | ||
| const response = await service.getMyEvents({ page: 1, limit: 10 }) |
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.
Avoid using useCustomerEventsService outside component context
The new reconcileByFetchingEvents action instantiates useCustomerEventsService(), but that composable calls useI18n() internally. useI18n requires an active Vue component instance, which is not present when reconcileByFetchingEvents runs (it is invoked from authActions.fetchBalance() and other event handlers after setup). This will throw getCurrentInstance()/useI18n errors as soon as fetchBalance is called, breaking balance refresh and the top‑up reconciliation flow. Consider moving the service call into a component/composable scope or refactoring the service to avoid useI18n when called from Pinia stores.
Useful? React with 👍 / 👎.
|
upstream changed, manually backporting |
Backport of #6500 onto
rh-test.Summary of changes
credit_addedaudit logs, emit telemetry on success, refresh balance, and clear pendingNotes
Validation
rh-testpnpm lint:fixandpnpm typechecklocallyPlease review backport diffs for any
rh-testbranch-specific interactions.┆Issue is synchronized with this Notion page by Unito