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

Update serializers.md #2215

Merged
merged 1 commit into from
Oct 31, 2017
Merged

Update serializers.md #2215

merged 1 commit into from
Oct 31, 2017

Conversation

stratigos
Copy link

  • Add note about ActionController::API and view_context set to nil, with respect to accessing helpers in a Serializer.

Purpose

Clarify a use case of scope that could present a problem when using a Rails 5 API controller. Refs #2144. If using the view_context for scope with an API controller, the view_context will always be nil, and helpers can not be accessed in a Serializer this way.

Changes

Added a note to the documentation, and demonstrated an example workaround.

Related GitHub issues

Additional helpful information

@@ -379,6 +379,31 @@ class PostsController < ActionController::Base
end
end
```
Note that `view_context` is made available through `ActionController::Base`. The `view_context` will be `nil` when accessed from a controller that inherits from `ActionController::API`. In such a case, a helper will need to be instantiated as needed, e.g.:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't there anything else that might substitute for view_context? Maybe helpers? or just self?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long story short - I can remove the example I posted if it helps keep the documentation more clear and coherent.

When I was working on this for my own project, I found that Rails.application.helpers only included the helpers defined by the current application, (i.e., helpers you wrote), and does not include other view helpers, like NumberHelper or CsrfHelper etc.

I only proposed a generic solution to imply that "some class that has these modules included needs instantiated, somehow" and that view_context wont include them.

I understand if the example I provided is out of scope for the documentation, and also something you'd rather not officially represent. Would it be simpler if I just removed the example, and only left the note itself? I.e.,

Note that `view_context` is made available through `ActionController::Base`. The `view_context` will be `nil` when accessed from a controller that inherits from `ActionController::API`.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we edit anywhere on the page that says view_context (for subclasses of ActionController:Base), and adds another example or note that any controller reference which provides the desired scope is acceptable, for example :self may be useful for subclasses of ActionController::API which does not include ActionView::ViewContext ?

Thoughts?

Copy link
Author

@stratigos stratigos Oct 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not sure if I follow the use of :self, as it too is always nil for:

class SomeApiController < ActionController::API
  serialization_scope :self
end

I think I can change the wording to match something similar to what you have above, though. I see what you mean generally - that just about any callable from the Controller can be passed into serialization_scope.

@bf4
Copy link
Member

bf4 commented Oct 30, 2017

Thanks! 💯 for the PR. Question about whether the proposed solution is best. See there.

We should add a Rails5 test for this, too, I think

@bf4
Copy link
Member

bf4 commented Oct 31, 2017

Awesome! Want to add yourself to the changelog?

@stratigos
Copy link
Author

@bf4 Hows it look now?

@bf4
Copy link
Member

bf4 commented Oct 31, 2017

Awesome! Want to add yourself to the changelog?

:) If not, I'll merge when it passes

@stratigos
Copy link
Author

stratigos commented Oct 31, 2017

Sorry, I had updated the git log instead ;)

 * Add note that any controller reference is acceptable for `serialization_scope`
 * Add note about `ActionController::API` and `view_context` set to `nil`, with respect to accessing helpers in a Serializer
 * refs rails-api#2144
 * Update CHANGELOG.md
@bf4 bf4 merged commit 0bbeeb3 into rails-api:0-10-stable Oct 31, 2017
@stratigos
Copy link
Author

Tysm! Happy to contribute something!

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.

2 participants