Fixed: gracefully handle unknown sources#59
Conversation
WalkthroughThe Stripe source creation service now handles unknown payment method types by creating a generic Spree::PaymentSource instead of raising an error. Corresponding test cases verify this fallback behavior for unsupported payment methods like cashapp and unknown types. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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.
🧹 Nitpick comments (1)
spec/services/spree_stripe/create_source_spec.rb (1)
235-247: Tighten assertions to enforce truly generic fallback.
be_a(Spree::PaymentSource)also matches subclasses, so these examples won’t fail if a specialized source class is returned later. Use exact-class checks here.Proposed spec tightening
- expect(subject).to be_a(Spree::PaymentSource) + expect(subject).to be_an_instance_of(Spree::PaymentSource) expect(subject.gateway_payment_profile_id).to eq source_id expect(subject.payment_method).to eq gateway @@ - expect(subject).to be_a(Spree::PaymentSource) + expect(subject).to be_an_instance_of(Spree::PaymentSource) expect(subject.gateway_payment_profile_id).to eq source_id expect(subject.payment_method).to eq gateway🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/services/spree_stripe/create_source_spec.rb` around lines 235 - 247, The spec uses loose type matching with expect(subject).to be_a(Spree::PaymentSource) which would also pass for subclasses; change these assertions to enforce an exact class match (e.g. use be_instance_of or compare subject.class == Spree::PaymentSource) in the examples that test the generic fallback (the examples in the 'if source is an unknown type' context and the earlier generic case), keeping the other expectations (gateway_payment_profile_id and payment_method) unchanged so the spec fails if a specialized source class is returned.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@spec/services/spree_stripe/create_source_spec.rb`:
- Around line 235-247: The spec uses loose type matching with expect(subject).to
be_a(Spree::PaymentSource) which would also pass for subclasses; change these
assertions to enforce an exact class match (e.g. use be_instance_of or compare
subject.class == Spree::PaymentSource) in the examples that test the generic
fallback (the examples in the 'if source is an unknown type' context and the
earlier generic case), keeping the other expectations
(gateway_payment_profile_id and payment_method) unchanged so the spec fails if a
specialized source class is returned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 197abf5c-0ff0-432e-8789-c4af68a0b1b7
📒 Files selected for processing (2)
app/services/spree_stripe/create_source.rbspec/services/spree_stripe/create_source_spec.rb
Summary by CodeRabbit