feat: Customer retrieve returns objec #2
Conversation
Introduces a test script to verify that sample virtual account data is correctly converted to a VirtualAccount object using BridgeApi::Util.convert_to_bridged_object. The script checks object type and accessor methods to ensure pattern matching works as expected.
WalkthroughModified virtual_account recognition in BridgeApi::Util to require Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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 |
bolom
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
test_virtual_account_pattern.rb (3)
7-29: Optional: Address RuboCop style conventions.RuboCop flags several style issues in the sample data:
- Use single quotes for string literals that don't need interpolation
- Add trailing commas in multiline hashes
These are minor style issues that can be addressed to maintain consistency with the codebase style guide.
Example fix for a few lines:
- id: "7f444e4d-9fbe-4a49-8f4c-cb084cf7ebde", - status: "activated", - developer_fee_percent: "0.0", + id: '7f444e4d-9fbe-4a49-8f4c-cb084cf7ebde', + status: 'activated', + developer_fee_percent: '0.0',
35-35: Consider testing pattern matching without resource_hint.The test explicitly uses
resource_hint: 'virtual_account', which bypasses pattern matching and forces the resource type. Consider adding a test case without the hint to verify that the new pattern inRESOURCE_PATTERNScorrectly identifies the object type.Add this test case after line 40:
# Test pattern matching without hint puts "Testing pattern matching WITHOUT resource_hint..." result_without_hint = BridgeApi::Util.convert_to_bridged_object(sample_data) puts "Result class without hint: #{result_without_hint.class}" if result_without_hint.is_a?(BridgeApi::Resources::VirtualAccount) puts "✅ Pattern matching works correctly!" else puts "❌ Pattern matching failed!" exit 1 end puts ""
42-57: Consider adding edge case tests.The current test validates a complete virtual_account object, but doesn't test the minimum requirements defined by the pattern. Consider adding tests for:
- Minimum required fields:
id+ only one of the "any" fields (e.g., justidandstatus)- Backward compatibility: Data with
account_numberbut missing new "any" fields- Negative case: Data with only
idbut none of the "any" fields (should not match)Example edge case test:
# Test minimum pattern requirements minimal_data = { id: 'test-id', status: 'activated' } minimal_result = BridgeApi::Util.convert_to_bridged_object(minimal_data, resource_hint: 'virtual_account') if minimal_result.is_a?(BridgeApi::Resources::VirtualAccount) puts "✅ Minimum required fields work correctly!" else puts "❌ Minimum required fields test failed!" exit 1 end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/bridge_api/util.rb(1 hunks)test_virtual_account_pattern.rb(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
test_virtual_account_pattern.rb (2)
lib/bridge_api/util.rb (1)
convert_to_bridged_object(28-40)lib/bridge_api/resources/virtual_account.rb (2)
developer_fee_percent(31-33)source_deposit_instructions(43-45)
🪛 RuboCop (1.81.7)
test_virtual_account_pattern.rb
[warning] 1-1: Script file test_virtual_account_pattern.rb doesn't have execute permission.
(Lint/ScriptPermission)
[convention] 8-8: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 9-9: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 10-10: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 11-11: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 12-12: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 14-14: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 15-15: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 16-16: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 17-17: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 18-18: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 19-19: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 20-20: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 21-21: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 22-22: Put a comma after the last item of a multiline hash.
(Style/TrailingCommaInHashLiteral)
[convention] 22-22: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 25-25: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 26-26: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 27-27: Put a comma after the last item of a multiline hash.
(Style/TrailingCommaInHashLiteral)
[convention] 27-27: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 28-28: Put a comma after the last item of a multiline hash.
(Style/TrailingCommaInHashLiteral)
[convention] 31-31: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 32-32: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 39-39: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 40-40: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 43-43: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 44-44: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 45-45: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 51-51: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 52-52: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
🔇 Additional comments (2)
test_virtual_account_pattern.rb (1)
1-57: Clarify test location and integration strategy.This test script is located at the repository root and uses a manual execution style rather than a testing framework. Please clarify:
- Is this script intended for temporary validation or permanent test coverage?
- If permanent, should it be:
- Moved to a
test/orspec/directory?- Converted to use the project's testing framework (RSpec/Minitest)?
- Integrated into CI/CD pipeline?
Also note: The PR title mentions "Customer retrieve returns object" but this script tests
virtual_accountpattern matching. Ensure the PR scope is clear.lib/bridge_api/util.rb (1)
20-20: The pattern change is correct and aligns with actual Bridge API responses. No backward compatibility concerns exist.Verification confirms:
account_numberdoes not appear in any Bridge API virtual account responses or anywhere in the codebase- The new pattern fields (
status,customer_id,source_deposit_instructions) are present in actual production API responses and supported by the VirtualAccount class- The pattern correctly reflects the actual API response structure used by the service
No action required.
Co-authored-by: Qwen-Coder <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
test_virtual_account_pattern.rb (1)
1-1: Execute permission already flagged.This concern was already raised in a previous review.
🧹 Nitpick comments (2)
test_virtual_account_pattern.rb (2)
7-57: Consider converting this to an automated test.This manual test script is useful for ad-hoc verification but would be more valuable as part of an automated test suite (e.g., RSpec or Minitest) that can run in CI/CD.
Example structure for an automated test:
# spec/util/virtual_account_conversion_spec.rb require 'spec_helper' RSpec.describe BridgeApi::Util do describe '.convert_to_bridged_object' do context 'with virtual account data' do let(:sample_data) do { id: '7f444e4d-9fbe-4a49-8f4c-cb084cf7ebde', status: 'activated', customer_id: 'f558b609-403f-4e88-8815-a1bc69c57159', source_deposit_instructions: { currency: 'eur' } } end it 'converts to VirtualAccount object' do result = described_class.convert_to_bridged_object( sample_data, resource_hint: 'virtual_account' ) expect(result).to be_a(BridgeApi::Resources::VirtualAccount) expect(result.id).to eq(sample_data[:id]) expect(result.status).to eq(sample_data[:status]) end end end end
8-52: Optional: Address RuboCop style conventions.RuboCop flagged several style conventions:
- Prefer single-quoted strings when interpolation isn't needed
- Add trailing commas in multiline hashes
These are purely stylistic and don't affect functionality. For test data resembling JSON structures, double quotes may actually be more readable. Consider addressing these only if you want strict RuboCop compliance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test_virtual_account_pattern.rb(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
test_virtual_account_pattern.rb (2)
lib/bridge_api/util.rb (1)
convert_to_bridged_object(28-40)lib/bridge_api/resources/virtual_account.rb (2)
developer_fee_percent(31-33)source_deposit_instructions(43-45)
🪛 RuboCop (1.81.7)
test_virtual_account_pattern.rb
[convention] 8-8: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 9-9: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 10-10: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 11-11: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 12-12: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 14-14: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 15-15: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 16-16: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 17-17: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 18-18: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 19-19: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 20-20: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 21-21: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 22-22: Put a comma after the last item of a multiline hash.
(Style/TrailingCommaInHashLiteral)
[convention] 22-22: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 25-25: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 26-26: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 27-27: Put a comma after the last item of a multiline hash.
(Style/TrailingCommaInHashLiteral)
[convention] 27-27: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 28-28: Put a comma after the last item of a multiline hash.
(Style/TrailingCommaInHashLiteral)
[convention] 31-31: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 32-32: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 39-39: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 40-40: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 43-43: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 44-44: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 45-45: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 51-51: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
[convention] 52-52: Prefer single-quoted strings when you don't need string interpolation or special symbols.
(Style/StringLiterals)
🔇 Additional comments (1)
test_virtual_account_pattern.rb (1)
4-4: Therequire_relative 'lib/bridge_api'is correct and idiomatic for this gem repository. The test file is in the repository root andlib/bridge_api.rbexists at the expected relative path. Gem test runners execute from the repository root by convention, so the relative path is not fragile. The suggested alternative usingFile.join(__dir__, ...)would be unnecessarily verbose with no functional improvement.Likely an incorrect or invalid review comment.
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.