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

Inconsistent ActiveResource::Base.build behavior: expects overriding attributes, instead takes prefix options #48

Closed
denzel-morris opened this issue Feb 5, 2013 · 1 comment

Comments

@denzel-morris
Copy link
Contributor

The Problem

The documentation says:

attributes - A hash that overrides the default values from the server.

But both the code (line 768 of base.rb)

def build(attributes = {})
  attrs = self.format.decode(connection.get("#{new_element_path(attributes)}", headers).body)
  self.new(attrs)
end

and the tests (line 669 of base_test.rb), say otherwise

def test_build_without_attributes_for_prefix_call
  ActiveResource::HttpMock.respond_to do |mock|
    mock.get "/people/1/addresses/new.json", {}, StreetAddress.new.to_json
  end
  assert_raise(ActiveResource::InvalidRequestError) { StreetAddress.build }
end
def test_build_with_attributes_for_prefix_call
  ActiveResource::HttpMock.respond_to do |mock|
    mock.get "/people/1/addresses/new.json", {}, StreetAddress.new.to_json
  end
  assert_nothing_raised { StreetAddress.build(person_id: 1) }
end

Attributes is passed to new_element_path which uses them as prefix options not as attributes to override the defaults provided.

Possible Solutions

  • Correct the behavior of build and use the attributes passed for a query parameter list instead of prefix options
  • Create a new method called initialize that takes attributes used to override the defaults and update the documentation for build

These are the two most viable solutions to me. There are others that just don't seem as great. Personally, I'm a fan of option 2.

I'd like to get some input before I modify the code, tests, and submit a pull request. Any suggestions?

@denzel-morris
Copy link
Contributor Author

Disregard this issue. The only thing that needs to be updated is the documentation for build; initialize already provides the exact behavior I was looking for.

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

1 participant