[CM-278] allow retrieve all sub merch

[jazziining]

Description:
This PR is to allow passing sub merchant id when calling the .show where we use the rpt_merchant_id like all other query elements. This will enable retrieving the report not only for a specific sub merchant but also for all live sub merchants.

This update should not affect any current usage, as you should still be able to pass the sub_merchant_id when calling the Bambora::Client.new. In the mean time, you can also get rid of that and pass additional element when calling the client.show where you can specify the sub merchant id by using the key rpt_merchant_id

Jira: CM-216

Test:

• I have added the test in the spec
• Test by adding the following in the console file and run ./bin/console

elements = {
  service_name: 'BatchPaymentsEFT',
  rpt_filter_by_1: 'returned_date',
  rpt_operation_type_1: 'GT',
  rpt_filter_value_1: '2023-08-01',
  rpt_merchant_id: 'AllLive',
}

client ||= begin
  url = ENV.fetch('BAMBORA_SCRIPTS_API_BASE_URL')
  partner_merchant_id = ENV.fetch("BAMBORA_CAD_MERCHANT_ID")
  reporting_api_key = ENV.fetch("BAMBORA_CAD_REPORTING_API_KEY")

  ::Bambora::Client
    .new(scripts_api_base_url: url, merchant_id: partner_merchant_id)
    .batch_reports(api_key: reporting_api_key)
end

response = client.show(elements)

puts response

• Test by adding the following in the console file and run ./bin/console

elements = {
  service_name: 'BatchPaymentsEFT',
  rpt_filter_by_1: 'batch_id',
  rpt_operation_type_1: 'EQ',
  rpt_filter_value_1: 10000004,
}

client ||= begin
  url = ENV.fetch('BAMBORA_SCRIPTS_API_BASE_URL')
  partner_merchant_id = ENV.fetch("BAMBORA_CAD_MERCHANT_ID")
  reporting_api_key = ENV.fetch("BAMBORA_CAD_REPORTING_API_KEY")

  ::Bambora::Client
    .new(scripts_api_base_url: url, merchant_id: partner_merchant_id, sub_merchant_id: sub_merchan_id)
    .batch_reports(api_key: reporting_api_key)
end

response = client.show(elements)

puts response

[jazziining]

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

[jazziining]

Remove this line as it is not used or needed.

[cristianoventura]

Looks good to me!

If we are planning to release this publicly right away I'd recommend bumping the patch or minor version as well as updating the changelog. If we do this, we probably need to make a release in ruby gems too, but if there are more related changes coming up, we can pack all of them together and release all at once.

[cristianoventura]

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

[cristianoventura]

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

[harrylewis]

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

[harrylewis]

@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?

[cristianoventura]

Suggested change

	let(:client_withot_sub_merchant_id) do
	let(:client_without_sub_merchant_id) do

[harrylewis]

@cristianoventura good call on the gem versioning. Since this is a change we are still going to be exercising and evaluating, I suggest we leave the versioning out for now until it has been living in production for some time, and then we can cut a new release.

Does that sound reasonable to you?

[harrylewis]

@cristianoventura @jazziining that being said, I think we could update the CHANGELOG, with a new Unreleased heading, with a point about this new change. Do you mind doing that Jasmine?

[harrylewis]

Oh wow, we can even keep the previous implementation mostly intact with that suggestion 🙏🏻

[harrylewis]

Small change, lots of testing, and a much more flexible API! Great work @jazziining!