Fix PayBridge gateway registration#56
Conversation
📝 WalkthroughWalkthroughThis PR refactors PayBridge SDK dependency injection to support configuration-based service registration alongside code-based customization, implements smart gateway selection that distinguishes between explicitly enabled and auto-detected gateways based on configured credentials, and adds supporting validation and null-safety improvements to the transaction and payment service layers. ChangesDI Configuration Binding and Smart Gateway Selection
Null-Safe Transaction Lookup and Empty Gateway Validation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
PayBridge.SDK.Example/Program.cs (1)
95-99:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the example comment to match the new auto-detection rule.
The SDK no longer enables every gateway whose section exists; it only enables gateways whose required credentials are populated with non-placeholder values. This example is the canonical setup guide, so the current wording will mislead anyone copying the sample config.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PayBridge.SDK.Example/Program.cs` around lines 95 - 99, Update the example comment to reflect the new auto-detection rule: explain that if EnabledGateways is empty the SDK will only enable gateways whose configuration sections contain real credentials (non-placeholder values), not merely those sections that exist; mention the "PaymentGatewayConfig:EnabledGateways" key in appsettings.json remains supported and that explicitly listing gateways is still recommended for production to avoid accidental enabling when credentials are left as placeholders.
🧹 Nitpick comments (1)
PayBridge.SDK.Test/Unit/IServiceCollectionExtensionsTests.cs (1)
44-64: ⚡ Quick winAdd one assertion for concrete-gateway resolution too.
This test proves
IPaymentGatewayregistration, but the new contract also promises direct injection of the concrete gateway type. AGetRequiredService<PeachPaymentsGateway>()assertion here would pin that behavior and catch regressions inAddGatewayRegistration<TGateway>().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@PayBridge.SDK.Test/Unit/IServiceCollectionExtensionsTests.cs` around lines 44 - 64, Update the test AddPayBridge_RegistersEnabledGateway_AsIPaymentGateway to also assert direct resolution of the concrete gateway type: after building the provider call provider.GetRequiredService<PeachPaymentsGateway>() and assert it is not null (or assert its GatewayType equals PaymentGatewayType.PeachPayments). This verifies the AddGatewayRegistration<TGateway>() behavior and will catch regressions where the concrete PeachPaymentsGateway is not registered for direct injection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@PayBridge.SDK/Externsions/IServiceCollectionExtensions.cs`:
- Around line 163-218: RegisterGateway currently ignores
PaymentGatewayType.Automatic (and any unknown enum values) causing silent no-op
registrations; update RegisterGateway to validate the incoming
PaymentGatewayType (from EnabledGateways) and throw an informative exception for
PaymentGatewayType.Automatic and any unsupported values instead of falling
through. Specifically, inside RegisterGateway (and before the switch or in a
default branch) detect PaymentGatewayType.Automatic and throw (or
ArgumentException) with a clear message referencing EnabledGateways and
PaymentGatewayType.Automatic, and ensure the switch has a default case that
throws for unknown enum values; keep using AddGatewayRegistration<TGateway> for
valid cases.
---
Outside diff comments:
In `@PayBridge.SDK.Example/Program.cs`:
- Around line 95-99: Update the example comment to reflect the new
auto-detection rule: explain that if EnabledGateways is empty the SDK will only
enable gateways whose configuration sections contain real credentials
(non-placeholder values), not merely those sections that exist; mention the
"PaymentGatewayConfig:EnabledGateways" key in appsettings.json remains supported
and that explicitly listing gateways is still recommended for production to
avoid accidental enabling when credentials are left as placeholders.
---
Nitpick comments:
In `@PayBridge.SDK.Test/Unit/IServiceCollectionExtensionsTests.cs`:
- Around line 44-64: Update the test
AddPayBridge_RegistersEnabledGateway_AsIPaymentGateway to also assert direct
resolution of the concrete gateway type: after building the provider call
provider.GetRequiredService<PeachPaymentsGateway>() and assert it is not null
(or assert its GatewayType equals PaymentGatewayType.PeachPayments). This
verifies the AddGatewayRegistration<TGateway>() behavior and will catch
regressions where the concrete PeachPaymentsGateway is not registered for direct
injection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bbfc9ed3-5788-4162-9de6-fc1680970b64
📒 Files selected for processing (6)
PayBridge.SDK.Example/Program.csPayBridge.SDK.Test/Unit/IServiceCollectionExtensionsTests.csPayBridge.SDK/Externsions/IServiceCollectionExtensions.csPayBridge.SDK/Interfaces/ITransactionRepository.csPayBridge.SDK/Repositories/TransactionRepository.csPayBridge.SDK/Services/PaymentService.cs
| foreach (var gateway in config.EnabledGateways) | ||
| { | ||
| switch (gateway) | ||
| { | ||
| case PaymentGatewayType.Stripe: | ||
| AddGatewayRegistration<StripeGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Paystack: | ||
| AddGatewayRegistration<PaystackGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Flutterwave: | ||
| AddGatewayRegistration<FlutterwaveGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Checkout: | ||
| AddGatewayRegistration<CheckoutGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.BenefitPay: | ||
| AddGatewayRegistration<BenefitPayGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Knet: | ||
| AddGatewayRegistration<KnetGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Monnify: | ||
| AddGatewayRegistration<MonnifyGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Squad: | ||
| AddGatewayRegistration<SquadGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Korapay: | ||
| AddGatewayRegistration<KorapayGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Interswitch: | ||
| services.AddScoped<InterswitchGateway>(); | ||
| services.AddScoped<IPaymentGateway>(sp => sp.GetRequiredService<InterswitchGateway>()); | ||
| break; | ||
| case PaymentGatewayType.Remita: | ||
| services.AddScoped<RemitaGateway>(); | ||
| break; | ||
| case PaymentGatewayType.Opay: | ||
| services.AddScoped<OpayGateway>(); | ||
| break; | ||
| case PaymentGatewayType.DpoGroup: | ||
| AddGatewayRegistration<DpoGroupGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.PawaPay: | ||
| services.AddScoped<PawaPayGateway>(); | ||
| break; | ||
| case PaymentGatewayType.PeachPayments: | ||
| services.AddScoped<PeachPaymentsGateway>(); | ||
| break; | ||
| } | ||
| RegisterGateway(services, gateway); | ||
| } | ||
| } | ||
|
|
||
| private static void RegisterGateway(IServiceCollection services, PaymentGatewayType gateway) | ||
| { | ||
| switch (gateway) | ||
| { | ||
| case PaymentGatewayType.Stripe: | ||
| AddGatewayRegistration<StripeGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Paystack: | ||
| AddGatewayRegistration<PaystackGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Flutterwave: | ||
| AddGatewayRegistration<FlutterwaveGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Checkout: | ||
| AddGatewayRegistration<CheckoutGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.BenefitPay: | ||
| AddGatewayRegistration<BenefitPayGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Knet: | ||
| AddGatewayRegistration<KnetGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Monnify: | ||
| AddGatewayRegistration<MonnifyGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Squad: | ||
| AddGatewayRegistration<SquadGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Korapay: | ||
| AddGatewayRegistration<KorapayGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Interswitch: | ||
| AddGatewayRegistration<InterswitchGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Remita: | ||
| AddGatewayRegistration<RemitaGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.Opay: | ||
| AddGatewayRegistration<OpayGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.DpoGroup: | ||
| AddGatewayRegistration<DpoGroupGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.PawaPay: | ||
| AddGatewayRegistration<PawaPayGateway>(services); | ||
| break; | ||
| case PaymentGatewayType.PeachPayments: | ||
| AddGatewayRegistration<PeachPaymentsGateway>(services); | ||
| break; | ||
| } |
There was a problem hiding this comment.
Fail fast when EnabledGateways contains Automatic.
RegisterGateway has no Automatic branch, so an explicit EnabledGateways: ["Automatic"] configuration succeeds at startup but registers nothing. That misconfiguration only shows up later when a payment is attempted. Reject Automatic (and ideally any unsupported enum value) in the explicit-registration path so the error is immediate.
Possible guard
foreach (var gateway in config.EnabledGateways)
{
+ if (gateway == PaymentGatewayType.Automatic)
+ {
+ throw new ArgumentException(
+ "EnabledGateways cannot contain Automatic. List concrete gateway types instead.",
+ nameof(config));
+ }
+
RegisterGateway(services, gateway);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@PayBridge.SDK/Externsions/IServiceCollectionExtensions.cs` around lines 163 -
218, RegisterGateway currently ignores PaymentGatewayType.Automatic (and any
unknown enum values) causing silent no-op registrations; update RegisterGateway
to validate the incoming PaymentGatewayType (from EnabledGateways) and throw an
informative exception for PaymentGatewayType.Automatic and any unsupported
values instead of falling through. Specifically, inside RegisterGateway (and
before the switch or in a default branch) detect PaymentGatewayType.Automatic
and throw (or ArgumentException) with a clear message referencing
EnabledGateways and PaymentGatewayType.Automatic, and ensure the switch has a
default case that throws for unknown enum values; keep using
AddGatewayRegistration<TGateway> for valid cases.
There was a problem hiding this comment.
@copilot What's the issue with this and can you fix it
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
Summary
Fixes PayBridge SDK registration so configuration binding, gateway DI visibility, and transaction lookup semantics match the behavior expected by the example app and payment service.
What changed
PaymentGatewayConfigfromPaymentGatewayConfigin configuration before applying code overrides.PaymentGatewayConfiginjection andIOptions<PaymentGatewayConfig>backed by the same object.IPaymentGateway.EnabledGatewaysis empty, register only gateways with configured non-placeholder credentials.GetByReferenceAsyncreturnnullwhen no transaction exists, matching existing service flow.Validation
dotnet test PayBridge.SDK.slnpassed: 90 passed, 2 skipped, 0 failed.Summary by CodeRabbit
New Features
Bug Fixes
Tests