-
Notifications
You must be signed in to change notification settings - Fork 240
chore(fiat-onramp): temporarily disable Ramp until API keys are regenerated #3015
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
WalkthroughThe changes temporarily disable the Ramp fiat provider and its associated payment methods in the application due to unavailable API keys. This is achieved by commenting out relevant code sections and adding explanatory comments and TODOs for re-enabling once the keys are available. No other logic or functionality is modified. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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. 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 (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lib/bloc/fiat/models/fiat_payment_method.dart (1)
123-145: Avoid leaving disabled code in-place – prefer a feature flag / build-time toggleKeeping >20 lines of commented-out code bloats the file and makes it harder to see real logic. Consider:
- Introduce a simple runtime flag (
kIsRampEnabled) that controls whether the twoFiatPaymentMethodentries are added.- Or move the Ramp methods to a separate file and exclude that file from the build via
dart define/ flavour specific imports.This keeps the main model tidy and prevents stale code from creeping in.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/bloc/fiat/models/fiat_payment_method.dart(1 hunks)lib/views/fiat/fiat_page.dart(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
lib/views/fiat/fiat_page.dart (2)
Learnt from: takenagain
PR: #2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: The excludedAssetList from app_config.dart is used in the _filterExcludedAssets method in coins_state.dart. Since coins_state.dart is a part file of coins_bloc.dart, the import needs to be in the parent file even though it's not directly used there. In Dart, part files share the namespace and imports of their parent files.
Learnt from: takenagain
PR: #2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: In the Komodo Wallet project, part files share imports with their parent files. The import for app_config.dart in coins_bloc.dart is necessary because the part file coins_state.dart uses excludedAssetList from that package.
lib/bloc/fiat/models/fiat_payment_method.dart (2)
Learnt from: takenagain
PR: #2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: The excludedAssetList from app_config.dart is used in the _filterExcludedAssets method in coins_state.dart. Since coins_state.dart is a part file of coins_bloc.dart, the import needs to be in the parent file even though it's not directly used there. In Dart, part files share the namespace and imports of their parent files.
Learnt from: takenagain
PR: #2566
File: lib/bloc/coins_bloc/coins_bloc.dart:10-10
Timestamp: 2025-04-01T15:51:37.060Z
Learning: In the Komodo Wallet project, part files share imports with their parent files. The import for app_config.dart in coins_bloc.dart is necessary because the part file coins_state.dart uses excludedAssetList from that package.
⏰ 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). (10)
- GitHub Check: build_and_preview
- GitHub Check: Build Mobile (iOS)
- GitHub Check: Build Mobile (Android)
- GitHub Check: validate_code_guidelines
- GitHub Check: Build Desktop (macos)
- GitHub Check: Build Desktop (linux)
- GitHub Check: Test web-app-macos
- GitHub Check: Test web-app-linux-profile
- GitHub Check: unit_tests
- GitHub Check: Build Desktop (windows)
| // Ramp API keys unavailable for the time being | ||
| // TODO(takenagain): re-enable when API keys are available | ||
| // [BanxaFiatProvider(), RampFiatProvider()], | ||
| [BanxaFiatProvider()], | ||
| coinsRepository, |
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.
ramp_fiat_provider.dart import is now unused – will fail on dart analyze --fatal-warnings
Since the provider list no longer contains RampFiatProvider, the import at line 11 becomes unused.
Either:
-import 'package:web_dex/bloc/fiat/ramp/ramp_fiat_provider.dart';
+// import removed while Ramp is disabled – add back when keys are refreshedor add the provider conditionally behind a compile-time flag so the import remains referenced when enabled.
Neglecting this will cause CI to break if unused-import warnings are treated as errors.
🤖 Prompt for AI Agents
In lib/views/fiat/fiat_page.dart around lines 51 to 55, the import for
ramp_fiat_provider.dart is now unused because RampFiatProvider is commented out
from the provider list. To fix this, either remove the import statement for
ramp_fiat_provider.dart at line 11 or reintroduce RampFiatProvider conditionally
using a compile-time flag so the import remains referenced when enabled. This
will prevent dart analyze from failing due to unused imports.
|
Visit the preview URL for this PR (updated for commit f1060e8): https://walletrc--pull-3015-merge-asg4ovmu.web.app (expires Tue, 05 Aug 2025 00:07:25 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
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.
Pull Request Overview
This PR temporarily disables Ramp fiat provider functionality due to unavailable API keys. The changes comment out Ramp-related payment methods and remove the Ramp provider from the active provider list until API keys can be refreshed.
- Removed RampFiatProvider from the active fiat providers list
- Commented out Ramp payment methods (Card Payment and Apple Pay)
- Added TODO comments for re-enabling once API keys are available
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| lib/views/fiat/fiat_page.dart | Removes RampFiatProvider from the FiatRepository initialization |
| lib/bloc/fiat/models/fiat_payment_method.dart | Comments out Ramp-specific payment methods (Card Payment and Apple Pay) |
CharlVS
left a comment
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.
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.
Since the RampFiatProvider() is disabled, are the changes to lib/bloc/fiat/models/fiat_payment_method.dart still necessary? I haven't looked at the full context yet, but I assume the available options are populated dynamically based on the registered providers.
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.
The defaultFiatPaymentMethods is a placeholder value used when
- there is no valid Fiat amount entered (initial form state), and
- when logging out and resetting the form (before updating providers with a default example fiat value of 10000).
Summary by CodeRabbit