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

Parse nested locales in AssetFields #66

Merged
merged 3 commits into from
Aug 25, 2015
Merged

Conversation

neonichu
Copy link
Contributor

Fixes #63

@neonichu
Copy link
Contributor Author

I have simply copied the magic from Contentful::Resource::Fields so that assets work the same way. I didn't fully understand why the split between the two was made, would obviously be nicer if both code paths would simply share this functionality.

@pxlpnk
Copy link
Contributor

pxlpnk commented Aug 24, 2015

Suggestion:
We pull this:

def extract_fields_from_object!(object)
@fields = {}
if nested_locale_fields?
object['fields'].each do |field_name, nested_child_object|
nested_child_object.each do |object_locale, real_child_object|
@fields[object_locale] ||= {}
@fields[object_locale].merge! extract_from_object(
{ field_name => real_child_object }, :fields
)
end
end
else
@fields[locale] = extract_from_object object['fields'], :fields
end
end

into the resource.rb module and make it available on the fields.rb and assets_fields.rb modules?
With this we can avoid code duplication as that code on its own is already rather complex.
WDYT?

@neonichu
Copy link
Contributor Author

@pxlpnk sounds good, will update the PR

@pxlpnk
Copy link
Contributor

pxlpnk commented Aug 25, 2015

Looks good!

pxlpnk added a commit that referenced this pull request Aug 25, 2015
@pxlpnk pxlpnk merged commit bf8d57d into master Aug 25, 2015
@pxlpnk pxlpnk deleted the parse-nested-locales-in-assets branch August 25, 2015 11:26
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