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

Adds support for the Address fields on Organisation #98

Merged
merged 2 commits into from Oct 18, 2016

Conversation

nikz
Copy link
Contributor

@nikz nikz commented Oct 16, 2016

No description provided.

@nikz nikz self-assigned this Oct 16, 2016
def to_xml_attributes(builder = Builder::XmlMarkup.new, path = nil, attr_definitions = self.class.attribute_definitions)
attr_definitions.each do |attr, value|
case value
when Hash
builder.__send__(attr) do
to_xml_attributes(builder, "#{path}#{attr}", value)
end
when Array
raise UnsupportedAttributeType.new("#{value} instances don't respond to #to_xml") unless value.first.method_defined?(:to_xml)
Copy link
Contributor

Choose a reason for hiding this comment

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

what if empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are we going to support not specifying the content of the array?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it works because nil.respond_to?(:to_xml) == true

org.to_xml
end
end

Copy link
Contributor

Choose a reason for hiding this comment

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

pretty sure current implementation will raise if org.addresses = [] ? maybe add a test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have added a test, not sure it does but maybe I'm missing the scenario you're thinking of?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it works because nil resonds to to_xml ?

@armstrjare armstrjare merged commit dfec20e into master Oct 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants