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_root_in_json true, but not in active_model_serializers_format or json_api_format problems #380

Open
fractaledmind opened this issue Oct 21, 2015 · 4 comments

Comments

@fractaledmind
Copy link

When moving a project from ActiveResource to her, my team and I came across a problem. The API we are using (which is being built by our team for an enterprise application) wraps the JSON response in a model name (e.g. GET /api/v1/accounts -> {"accounts":[<data>]}, or GET /api/v1/accounts/2 -> {"account":{<data>}}). This API, however, is not using ActiveModelSerializer to generate the responses (it uses JBuilder templates, FWIW). When creating the models to be backed by her, we simply had:

class Account
  include Her::Model
  parse_root_in_json true
end

Attempting to access that model threw an error, however: Undefined method '.keys' for #<Array>. In debugging this issue, I found that the problem lied in the extract_array method in the parse.rb file:

def extract_array(request_data)
  if request_data[:data].is_a?(Hash) && (active_model_serializers_format? || json_api_format?)
    request_data[:data][pluralized_parsed_root_element]
  else
    request_data[:data]
  end
end

The issue is that our request_data wasn't passing the first if clause, since I didn't state that the format was one of those two options, although the response was a Hash. This method thus simply returned the hash response (i.e. accounts: {<data>}), instead of accessing the nested data.

There are obviously numerous simple fixes (remove the second half of the if check, move that second half into a new nested if check, if Hash, try to access request_data[:data][pluralized_parsed_root_element] and handle possible errors, etc), but I don't presume to understand precisely what your large scale project decisions are that led to this particular implementation, thus I haven't yet put in a Pull Request.

Hopefully, this is clear enough to display the issue that my team and I had. If not, let me know what else would prove valuable to you.

Love the project!

stephen

@hubert
Copy link
Contributor

hubert commented Oct 25, 2015

Hi Stephen,

Thanks for writing in with your issue. I think the best course of action to
your problem would be to specify that the api use the
active_model_serializers format. I believe that should interpret the root
keys for your data correctly.

The naming is probably a bit misleading since it indicates that one should
specify the format to consume an API that is produced by
active_model_serializers when what is really specifying is what is expected
for the root keys.

Let me know if this fixes your issue.

Thanks!

On Wed, Oct 21, 2015 at 8:55 AM, Stephen Margheim [email protected]
wrote:

When moving a project from ActiveResource to her, my team and I came
across a problem. The API we are using (which is being built by our team
for an enterprise application) wraps the JSON response in a model name
(e.g. GET /api/v1/accounts -> {"accounts":[]}, or GET
/api/v1/accounts/2 -> {"account":{}}). This API, however, is not
using ActiveModelSerializer to generate the responses (it uses JBuilder
templates, FWIW). When creating the models to be backed by her, we simply
had:

class Account
include Her::Model
parse_root_in_json true
end

Attempting to access that model threw an error, however: Undefined method
'.keys' for #. In debugging this issue, I found that the problem
lied in the extract_array method in the parse.rb file:

def extract_array(request_data)
if request_data[:data].is_a?(Hash) && (active_model_serializers_format? || json_api_format?)
request_data[:data][pluralized_parsed_root_element]
else
request_data[:data]
end
end

The issue is that our request_data wasn't passing the first if clause,
since I didn't state that the format was one of those two options, although
the response was a Hash. This method thus simply returned the hash
response (i.e. accounts: {}), instead of accessing the nested data.

There are obviously numerous simple fixes (remove the second half of the
if check, move that second half into a new nested if check, if Hash, try
to access request_data[:data][pluralized_parsed_root_element] and handle
possible errors, etc), but I don't presume to understand precisely what
your large scale project decisions are that led to this particular
implementation, thus I haven't yet put in a Pull Request.

Hopefully, this is clear enough to display the issue that my team and I
had. If not, let me know what else would prove valuable to you.

Love the project!

stephen


Reply to this email directly or view it on GitHub
#380.

@fractaledmind
Copy link
Author

@hubert That does fix my problem, but I'm not certain it's the best general fix. What is the point of parse_root_in_json true if it doesn't actually parse the root key without that format specified? Would it not be possible to add another else path to the extract_array function for the situation where parse_root_in_json is true, but no format is specified? If the request_data[:data][pluralized_parsed_root_element] fails, raise an exception that suggests manually specifying the root key.

If this seems acceptable, I can put in a PR.

@mikenichols
Copy link

I ran into this same problem and I struggled with it for a while. I was eventually able to get it working by explicitly specifying: parse_root_in_json true, format: :active_model_serializers. I like @smargh 's idea, it should try something sane.

@fractaledmind
Copy link
Author

I may just put in a PR with this suggested fix.

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

No branches or pull requests

3 participants