Skip to content
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.

Sign up for GitHub

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

[CM-278] allow retrieve all sub merch #50

Merged
merged 5 commits into from
Dec 1, 2023
Merged

[CM-278] allow retrieve all sub merch #50

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
452e660
Set sub merchant to alllive if not available
jazziining Nov 28, 2023
242171f
:remove the sub_merchant_id from the client
jazziining Nov 30, 2023
e726079
Add tests
jazziining Dec 1, 2023
5eb1707
Update changelog
jazziining Dec 1, 2023
022c112
Fix pr review
jazziining Dec 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions lib/bambora/bank/batch_report_resource.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,14 @@ def add_messages_to_response(response)
end

def batch_report_body(request_data)
DEFAULT_REQUEST_PARAMS.merge(request_data).merge(
additional_data = {
merchant_id: client.merchant_id,
pass_code: api_key,
sub_merchant_id: client.sub_merchant_id,
)
}

params = DEFAULT_REQUEST_PARAMS.merge(additional_data).merge(request_data)

client.sub_merchant_id.nil? ? params : params.merge(sub_merchant_id: client.sub_merchant_id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocker, but an alternative could be using .compact to remove sub_merchant_id if it's nil.

https://ruby-doc.org/core-2.4.2/Hash.html#method-i-compact

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, does this new behaviour need to be documented in the Readme?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The suggestion from @cristianoventura is really good here. It may remove the need for the conditional logic, as long as we change the merge order, and then call #compact.

additional_data = {
  merchant_id: client.merchant_id,
  pass_code: api_key,
  sub_merchant_id: client.sub_merchant_id,
}

DEFAULT_REQUEST_PARAMS.merge(additional_data).merge(request_data).compact

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cristianoventura that's a good idea to update the README, although it needs some work in general. I'll note this for future work to update the README. Is that cool with you?

end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/bambora/client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ def bank_profiles(api_key:)
#
# @param api_key [String] API key for the bank profiles endpoint.
#
# @return [Bambora::Bank::PaymentProfileResource]
# @return [Bambora::Bank::BatchReportResource]
def batch_reports(api_key:)
@batch_reports = Bambora::Bank::BatchReportResource.new(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change here as you can see from the line 168 the expected return is Bambora::Bank::BatchReportResource not Bambora::Bank::PaymentProfileResource

client: xml_client,
Expand Down
52 changes: 51 additions & 1 deletion spec/bambora/bank/batch_report_resource_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@ module Bambora
module Bank
describe BatchReportResource do
subject(:reports) { described_class.new(client: client, api_key: api_key) }
subject(:reports_without_sub_merchant_id) do
described_class.new(client: client_withot_sub_merchant_id, api_key: api_key)
end

let(:api_key) { 'fakekey' }
let(:merchant_id) { 1 }
let(:sub_merchant_id) { 2 }
let(:base_url) { 'https://sandbox-web.na.bambora.com' }
let(:headers) { { 'Authorization' => 'Passcode MTpmYWtla2V5', 'Sub-Merchant-ID' => sub_merchant_id } }
Copy link
Contributor Author

@jazziining jazziining Dec 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this line as it is not used or needed.


let(:response_body) do
{
response: {
Expand Down Expand Up @@ -96,6 +99,15 @@ module Bank
)
end

let(:client_withot_sub_merchant_id) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let(:client_withot_sub_merchant_id) do
let(:client_without_sub_merchant_id) do

instance_double(
'Bambora::Rest::XMLClient',
merchant_id: merchant_id,
sub_merchant_id: nil,
post: response_body,
)
end

describe '#show' do
let(:batch_id) { 1 }
let(:from_date) { Time.now.to_s }
Expand All @@ -112,6 +124,7 @@ module Bank
service_name: service_name,
}
end

let(:posted_data) do
{
body: {
Expand All @@ -132,6 +145,34 @@ module Bank
}
end

let(:request_data_with_rpt_merchant_id) do
{
service_name: service_name,
rpt_filter_by_1: 'returned_date',
rpt_operation_type_1: 'GT',
rpt_filter_value_1: '2023-08-01',
rpt_merchant_id: 'AllLive',
}
end

let(:posted_data_with_rpt_merchant_id) do
{
body: {
merchant_id: 1,
pass_code: 'fakekey',
rpt_filter_by_1: 'returned_date',
rpt_filter_value_1: '2023-08-01',
rpt_format: 'JSON',
rpt_operation_type_1: 'GT',
rpt_version: '2.0',
rpt_merchant_id: 'AllLive',
service_name: service_name,
session_source: 'external',
},
path: '/scripts/reporting/report.aspx',
}
end

context 'with no messageId' do
it 'sends `post` to the client with the correct data' do
reports.show(request_data)
Expand All @@ -141,6 +182,15 @@ module Bank
it 'returns the expected response' do
expect(reports.show(request_data)).to eq expected_response
end

it 'sends `post` to the client with the correct data where the sub merchant id comes from elements' do
reports_without_sub_merchant_id.show(request_data_with_rpt_merchant_id)
expect(client_withot_sub_merchant_id).to have_received(:post).with(posted_data_with_rpt_merchant_id)
end

it 'returns the expected response' do
expect(reports_without_sub_merchant_id.show(request_data_with_rpt_merchant_id)).to eq expected_response
end
end

context 'with messageIds' do
Expand Down
Loading