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

belongs_to associations no longer respect collection_paths #513

Closed
keegangroth opened this issue Oct 20, 2018 · 4 comments · Fixed by #514
Closed

belongs_to associations no longer respect collection_paths #513

keegangroth opened this issue Oct 20, 2018 · 4 comments · Fixed by #514

Comments

@keegangroth
Copy link
Contributor

The change in version 1.0.3 caused a regression in the handling of paths for the belongs_to association. It accepts an optional path now, but the default value for that option ignores the associated class's collection path. Simple broken example:

class Company
  include Her::Model
  include_root_in_json true
  collection_path 'my_companies'
end

class Admin
  include Her::Model
  include_root_in_json true
  belongs_to :company
end

in order to make the same thing work with 1.0.3, you would have to write belongs_to :company, path: "#{Company.collection_path}/:company_id" in the admin class.

One simple solution is to not provide a default value for the path argument.

@keegangroth
Copy link
Contributor Author

just to be clear, this was caused by #511

@j1mmie
Copy link

j1mmie commented Jan 23, 2019

I see this is fixed in #514 - any chance #514 will be merged?

@zacharywelch
Copy link
Collaborator

zacharywelch commented Feb 11, 2019

Confirmed this is a regression 😅

Although documentation states

Options Hash (opts):
:path (Path) — The relative path where to fetch the data (defaults to /class_name.pluralize/id)

In the end we were actually defaulting to resource_path or collection_path. Go figure 😉

The ideal solution would continue to support the custom path option as well as default to resource_path or collection_path based on the type of association. Keep in mind we also maintain a list of associations on the parent class. Specs for that can be found here FWIW #425 documents a similar issue with has_many.

zacharywelch pushed a commit that referenced this issue Feb 13, 2019
…514)

* belongs_to association should respect association's collection_path

fixes #513
@zacharywelch
Copy link
Collaborator

@keegangroth's fix can be found in release v1.1.0. Thanks everyone!

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 a pull request may close this issue.

3 participants