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.

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

Service setting attributes added to api response #837

Merged
merged 1 commit into from Jun 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 16 additions & 0 deletions app/models/service.rb
Expand Up @@ -367,6 +367,22 @@ def to_xml(options = {})
xml.backend_version proxy&.authentication_method
xml.description description

xml.intentions_required intentions_required
xml.buyers_manage_apps buyers_manage_apps
xml.buyers_manage_keys buyers_manage_keys
xml.referrer_filters_required referrer_filters_required
xml.custom_keys_enabled custom_keys_enabled
xml.buyer_key_regenerate_enabled buyer_key_regenerate_enabled
xml.mandatory_app_key mandatory_app_key
xml.buyer_can_select_plan buyer_can_select_plan
xml.buyer_plan_change_permission buyer_plan_change_permission

if notification_settings
xml.notification_settings do |xml|
notification_settings.each { |notification, values| xml.tag! notification, values }
end
end

xml.deployment_option deployment_option
xml.support_email support_email

Expand Down
15 changes: 15 additions & 0 deletions app/representers/service_representer.rb
Expand Up @@ -14,6 +14,17 @@ module ServiceRepresenter
property :support_email
property :description

property :intentions_required
property :buyers_manage_apps
property :buyers_manage_keys
property :referrer_filters_required
property :custom_keys_enabled
property :buyer_key_regenerate_enabled
property :mandatory_app_key
property :buyer_can_select_plan
property :buyer_plan_change_permission
property :notification_settings

property :created_at
property :updated_at

Expand Down Expand Up @@ -44,4 +55,8 @@ module ServiceRepresenter
def backend_version
proxy&.authentication_method
end

def notification_settings
attributes['notification_settings']&.stringify_keys
end
end
24 changes: 21 additions & 3 deletions spec/acceptance/api/service_spec.rb
@@ -1,8 +1,21 @@
# frozen_string_literal: true

require 'rails_helper'

resource "Service" do

let(:resource) { FactoryBot.build(:service, account: provider, system_name: 'foobar') }
let(:resource) { FactoryBot.build(:service,
account: provider,
system_name: 'foobar',
buyer_plan_change_permission: 'request_credit_card',
notification_settings: {
web_provider: ['', '50', '100', '300'],
email_provider: ['', '50', '100', '150'],
web_buyer: ['', '50', '100', '150'],
email_buyer: ['', '50', '100', '300']
}
)}
let(:attributes) { %w[id system_name intentions_required buyers_manage_apps buyers_manage_keys referrer_filters_required custom_keys_enabled buyer_key_regenerate_enabled mandatory_app_key buyer_can_select_plan buyer_plan_change_permission] }

before do
provider.settings.allow_multiple_services!
Expand Down Expand Up @@ -40,7 +53,11 @@

context 'service' do
subject(:service) { Hash.from_xml(serialized).fetch(root) }
it { should include('id' => resource.id.to_s, 'system_name' => resource.system_name) }
it { should include(attributes.map do |attr_name|
next if (attr_value = resource.public_send(attr_name)).blank?
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't need this line having the compact later, no? It should work like this:

it { should include(attributes.map { |attr_name| [attr_name, resource.public_send(attr_name).to_s.presence] }.compact.to_h) }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes and no. If we just collect the pairs without skipping when blank and without compact, it will work for this test in particular. However it's more about communicating through what's expected. If an attribute of resource is blank, it won't be included. I know, I should have added another test, but it was never there even though it's been the behavior expected since ever.

[attr_name, attr_value.to_s]
end.compact.to_h)}
it { should include('notification_settings' => resource.notification_settings.stringify_keys.transform_values(&:to_s)) }
end
end

Expand All @@ -49,7 +66,8 @@

let(:root) { 'service' }

it { should include('id' => resource.id, 'system_name' => resource.system_name) }
it { should include(attributes.map { |attr_name| [attr_name, resource.public_send(attr_name)] }.to_h.delete_if { |k, v| v.nil? })}
it { should include('notification_settings' => resource.notification_settings.stringify_keys) }
it { should have_links(%w|self end_user_plans service_plans application_plans features metrics|)}
end

Expand Down