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

Prevent non-resource backed controller from throwing no method error when mix-in is inherited from ApplicationController #46

Conversation

daniel-sullivan
Copy link

Change to allow 'include JsonapiSuite::ControllerMixin' to be called directly from ApplicationController without needing to specify 'jsonapi resource: < name >' in every descendant controller.

Fix for #45

…directly from ApplicationController without needing to specify 'jsonapi resource: < name >' in every descendant controller.

Fix for jsonapi-suite#45
@richmolj
Copy link
Contributor

Hey @daniel-sullivan, thanks for this PR! I'm not sure if it's the way I would go, though. If you have controllers without resources, these are likely not JSONAPI endpoints. If they are not JSONAPI endpoints, I would split my controllers to make this explicit:

class ApplicationController < ActionController::API
end

class JSONAPIController < ApplicationController
  include JsonapiSuite::ControllerMixin
end

class EmployeesController < JSONAPIController
  jsonapi resource: EmployeeResource
end

class PostsController < ApplicationController
  # no jsonapi here
end

This treats the situation more as a ruby issue than an issue specific to JSONAPI Suite. Would that work for you?

@daniel-sullivan
Copy link
Author

I agree and I have moved to doing it that way. The initial misconception came from when I was originally following the tutorial (this has since changed and I can't find the reference anywhere), the recommendation was to do the definition in application.rb. In my case where this is an existing project, that generated all sorts of errors on controllers that weren't being used for the API functions.

Now that those references have gone (if they even existed at all - it may have been a fever dream, I've been working a lot of overtime recently...), there's really nothing to get confused about so I'm going to close this and #46

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