Add serialization_scope example #1252

bf4 · 2015-10-07T10:28:26Z

beauby · 2015-10-07T14:58:34Z

This is important but I find the example confusing.

bf4 · 2015-10-08T19:20:43Z

README.md

+  def index
+    render json: User.new(id: 1, name: 'Pete', admin: false), serializer: AdminUserSerializer, adapter: :json_api
+  end
+end


@beauby how could the example be better. I was lazy and just cribbed the test, but maybe you have something in mind?

@bf4: Could you fill me in on what exactly we are testing here? It feels like we're making sure we can override the scope?

Not testing, documenting usage (no overriding)

B mobile phone

On Oct 8, 2015, at 5:27 PM, Lucas Hosseini [email protected] wrote:

In README.md:

++ +In order to do this, all we need to do is declare theserialization_scope: + +ruby
+class SomeController < ActionController::Base

serialization_scope :current_admin

def current_admin

User.new(id: 2, name: 'Bob', admin: true)

end

def index

render json: User.new(id: 1, name: 'Pete', admin: false), serializer: AdminUserSerializer, adapter: :json_api

end
+end
@bf4: Could you fill me in on what exactly we are testing here? It feels like we're making sure we can override the scope?

—
Reply to this email directly or view it on GitHub.

Haha right, I was a bit confused. What do you think about:

class PostSerializer < AMS attributes :title, :body, :can_edit? def can_edit? scope.admin? end end

sure, wanna edit? I'm busy for a while

bf4 · 2015-10-20T17:59:41Z

ref: #1281 (comment)

bf4 · 2015-10-21T21:34:55Z

Updated, but the example needs help from someone who's used the feature

bf4 · 2015-10-22T22:21:32Z

@rails-api/ams any thoughts on this?

NullVoxPopuli · 2015-10-22T23:29:21Z

I think the example is good. If there are no other objects, I'll merge it

beauby · 2015-10-26T00:48:57Z

README.md

+
+In the usual case, the serializer instance method has the same name as the controller method.
+e.g. `serializer.current_user` calls `controller.current_user`. The method's name
+will always be the `serialization_scope` defined in the controller, but may be


This sentence is a bit hard to understand (at least for me). What is "the serialization_scope"? The parameter to the (class) method call serialization_scope :foo? (i.e. :foo in this case) I guess not as you talk about the controller.

So, here's my dirty secret: this part of the code is a bit hard to understand.. by putting something in master, it forces a few more questions and I'm hoping maybe someone else can may it clearer :)

👍 I'm on board with that.

bf4 · 2015-11-20T21:24:53Z

omg, great examples in https://github.com/rails-api/active_model_serializers/tree/0-9-stable#customizing-scope and see filter https://github.com/rails-api/active_model_serializers/tree/0-9-stable#attributes

bf4 · 2015-11-26T05:41:00Z

Found another related issue: #741

It references an example setting serialization_scope to view_context and http://railscasts.com/episodes/409-active-model-serializers?view=asciicast

Our serializer is now working well but we have some issues with how the scope works. One problem is that it loads the current user record every time it makes a JSON request in our application, even if the user record isn’t accessed in the serializer. This can result in unnecessary database queries and potential performance issues. Another issue is that the name scope is rather generic and it’s not really obvious that the admin? method is called on the current user when we call it on scope. It would be much better if we could call current_user directly in our serializer. To get this to work we can customize the scope object that’s passed in to our serializers by changing the ApplicationController, calling serialization_scope and telling it to use something other than the current user, such as a view_context.

/app/controllers/application_controller.rb
serialization_scope :view_context
We can now call the current user through this view context and any other helper methods that we might need to access within our serializer. We’ll go back to our serializer and tell it to delegate the current_user method to the scope and then call admin? on that.

/app/serializers/article_serializer.rb
delegate :current_user, to: :scope
def attributes
  data = super
  data[:edit_url] = edit_article_url(object) if current_user.admin?
  data
end
Our page now has the same functionality as it did before but the current user is only loaded in as needed. One downside of this approach is that it can make testing a little more difficult as we need to provide access to the entire view context for the serializer. To get around this we can test it in a similar way to how we test helper methods by inheriting from ActionView::TestCase. This will automatically set up the view context for us so that we can pass it into the serializer.

bf4 · 2016-01-14T05:01:38Z

docs/general/serializers.md

+| options | `Serializer#scope` | method definition |
+|-------- | ------------------|
+| `scope: :current_user, scope_name: :current_user` | `:current_user` | `Serializer#current_user` calls `controller.current_user`
+| `scope: :current_admin, scope_name: :current_admin` | `:current_admin` | `Serializer#current_admin` calls `controller.current_admin`


I think there's bugs hiding here

bf4 · 2016-01-14T05:04:14Z

I updated with some more thought and comparison to 0.9. I like the examples better, and now understand the discussion of 'authorization context'

I think we lost the usage of the scope option in the controller, need to confirm with tests.

e.g. <%= react_component('CommunityRouter', posts: ActiveModel::SerializableResource.new(@posts, scope: current_business).serializable_hash ) %>

Also, looks like the serializer could break if scope_name is passed in but not a scope.

buren · 2016-02-08T08:30:51Z

I think in general this documentation is good and a well needed addition :) 👍 @bf4

However I also noticed some problems trying to pass in scope in controllers. I had some other problems there (in my app-code..), so will need to confirm that that is actually happening.. Will update when/if I can confirm anything.

bf4 · 2016-02-08T14:49:10Z

Feel free to make a pr into my branch

B mobile phone

On Feb 8, 2016, at 2:31 AM, Jacob Burenstam [email protected] wrote:

I think in general this documentation is good and a well needed addition :) @bf4

However I also noticed some problems trying to pass in scope in controllers. I had some other problems there (in my code..), so will need to confirm that that is actually happening.. Will update when/if I can confirm anything.

—
Reply to this email directly or view it on GitHub.

bf4 · 2016-02-10T06:45:30Z

@buren I may have found the bug

bf4 · 2016-02-10T06:59:14Z

docs/general/serializers.md

+
+In the controller, the scope/scope_name options are equal to
+the [`serialization_scope`method](https://github.com/rails-api/active_model_serializers/blob/d02cd30fe55a3ea85e1d351b6e039620903c1871/lib/action_controller/serialization.rb#L13-L20),
+which is `:current_user`, by default.


The controller defineds

class_attribute :_serialization_scope self._serialization_scope = :current_user def serialization_scope send(_serialization_scope) if _serialization_scope && respond_to?(_serialization_scope, true) end def serialization_scope(scope) self._serialization_scope = scope end end

then serialization resource calls

serializable_resource.serialization_scope ||= serialization_scope serializable_resource.serialization_scope_name = _serialization_scope

notice that serialization_scope is actually the return value of sending the _serialization_scope method in the controller (if that method exists).

Those two setters correspond to the serializar options scope and scope_name respectively, which may also be passed in as params to render

bf4 · 2016-02-11T03:27:07Z

test/action_controller/serialization_scope_name_test.rb

+        get :render_post_with_no_scope
+      end
+      assert_match exception_matcher, exception.message
+    end


Bug in Serializer._serializer_instance_methods is causing the scoped method to sometimes be available to read_attribute_for_serialization and sometime not, since the public instance methods are cached once, but they change with scopes.

is the fix for this bug for another PR?

bf4 · 2016-02-11T04:44:48Z

docs/general/serializers.md

+
+  # scope comments to those created_by the current user
+  has_many :comments do
+    object.comments.where(created_by: current_user)


is same as object.comments.where(created_by: scope)

bf4 · 2016-02-11T05:10:03Z

docs/general/serializers.md

+present and the controller responds to `scope_name`.
+
+Thus, in a serializer, the controller provides `current_user` as the
+current authorization scope when you call `render :json`.


There is currently a bug that prevents you from passing scope or scope_name via render options.

buren · 2016-02-11T07:04:38Z

@bf4 wow great work! Impressed that you found the "special" Rails 4 behaviour 👍

bf4 · 2016-02-17T16:49:17Z

Impressed that you found the "special" Rails 4 behaviour

@buren well, it failed tests.. so..

NullVoxPopuli · 2016-03-09T02:38:30Z

👍

NoMethodError is current_user is nil, so nil.admin? NameError is a superclass of NoMethodError (which Rails 4.0 won't allow) and means current_user might not be defined

[DOCS/TEST] Add serialization_scope example

bf4 added V: 0.10.x C: Documentation labels Oct 7, 2015

bf4 reviewed Oct 8, 2015
View reviewed changes

bf4 mentioned this pull request Oct 20, 2015

Is it possible to pass outside variables into the serializer? #1278

Closed

bf4 force-pushed the document_serialization_scope branch 2 times, most recently from bb4383c to 6b6aed8 Compare October 21, 2015 21:34

bf4 force-pushed the document_serialization_scope branch from 6b6aed8 to 4000b92 Compare October 21, 2015 21:50

beauby reviewed Oct 26, 2015
View reviewed changes

bf4 mentioned this pull request Oct 30, 2015

Add controller lookup. #1265

Closed

bf4 added the S: Needs Submitter Response label Nov 24, 2015

bf4 mentioned this pull request Nov 26, 2015

Add serialization_scope to README #741

Closed

bf4 mentioned this pull request Dec 14, 2015

support read_multi #1372

Merged

bf4 mentioned this pull request Jan 13, 2016

Conditional attributes/associations (if/unless) #1403

Merged

bf4 force-pushed the document_serialization_scope branch from 4000b92 to c36271a Compare January 14, 2016 05:01

bf4 reviewed Jan 14, 2016
View reviewed changes

bf4 force-pushed the document_serialization_scope branch from c36271a to 619807c Compare January 14, 2016 05:05

bf4 mentioned this pull request Feb 10, 2016

Optional attributes / Passing arbitrary params to serializer #599

Closed

bf4 force-pushed the document_serialization_scope branch from 619807c to fd544ad Compare February 10, 2016 06:44

bf4 reviewed Feb 10, 2016
View reviewed changes

bf4 force-pushed the document_serialization_scope branch from fd544ad to 002292a Compare February 11, 2016 02:29

bf4 reviewed Feb 11, 2016
View reviewed changes

bf4 force-pushed the document_serialization_scope branch from 47669f5 to 12ce44d Compare February 11, 2016 04:41

bf4 reviewed Feb 11, 2016
View reviewed changes

bf4 force-pushed the document_serialization_scope branch from 12ce44d to 5dd6f34 Compare February 11, 2016 05:08

bf4 reviewed Feb 11, 2016
View reviewed changes

bf4 force-pushed the document_serialization_scope branch from 5dd6f34 to 2958fbf Compare February 11, 2016 05:12

bf4 added S: Needs Code Review and removed S: Needs Submitter Response labels Feb 17, 2016

This was referenced Feb 29, 2016

1506 document passing arbitrary options to serializer #1545

Merged

Document passing arbitrary options to serializer #1506

Closed

bf4 force-pushed the document_serialization_scope branch from 2958fbf to 364b8ab Compare March 10, 2016 05:23

bf4 added 3 commits March 12, 2016 19:59

Add serialization_scope example [ci skip]

26b089c

Add better serialization_scope tests; uncover bug

85658c0

Differentiate exception behavior in Rails 4.0 vs. others

1b2f5ec

NoMethodError is current_user is nil, so nil.admin? NameError is a superclass of NoMethodError (which Rails 4.0 won't allow) and means current_user might not be defined

bf4 force-pushed the document_serialization_scope branch from 364b8ab to 1b2f5ec Compare March 13, 2016 02:00

bf4 added a commit that referenced this pull request Mar 13, 2016

Merge pull request #1252 from bf4/document_serialization_scope

3e0f85e

[DOCS/TEST] Add serialization_scope example

bf4 merged commit 3e0f85e into rails-api:master Mar 13, 2016

bf4 deleted the document_serialization_scope branch March 13, 2016 04:37

bf4 added the S: Needs followup label Mar 13, 2016

ZempTime mentioned this pull request Sep 4, 2017

Defining different sets of rules on how to serialize data in the same serializer [brainstorm/proposal] #2186

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add serialization_scope example #1252

Add serialization_scope example #1252

bf4 commented Oct 7, 2015

beauby commented Oct 7, 2015

bf4 Oct 8, 2015

beauby Oct 8, 2015

bf4 Oct 9, 2015

beauby Oct 9, 2015

bf4 Oct 9, 2015

bf4 commented Oct 20, 2015

bf4 commented Oct 21, 2015

bf4 commented Oct 22, 2015

NullVoxPopuli commented Oct 22, 2015

beauby Oct 26, 2015

bf4 Oct 26, 2015

NullVoxPopuli Oct 26, 2015

bf4 commented Nov 20, 2015

bf4 commented Nov 26, 2015

bf4 Jan 14, 2016

bf4 commented Jan 14, 2016

buren commented Feb 8, 2016

bf4 commented Feb 8, 2016

bf4 commented Feb 10, 2016

bf4 Feb 10, 2016

bf4 Feb 11, 2016

NullVoxPopuli Mar 9, 2016

bf4 Feb 11, 2016

bf4 Feb 11, 2016

buren commented Feb 11, 2016

bf4 commented Feb 17, 2016

NullVoxPopuli commented Mar 9, 2016

Add serialization_scope example #1252

Add serialization_scope example #1252

Conversation

bf4 commented Oct 7, 2015

beauby commented Oct 7, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bf4 commented Oct 20, 2015

bf4 commented Oct 21, 2015

bf4 commented Oct 22, 2015

NullVoxPopuli commented Oct 22, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bf4 commented Nov 20, 2015

bf4 commented Nov 26, 2015

Choose a reason for hiding this comment

bf4 commented Jan 14, 2016

buren commented Feb 8, 2016

bf4 commented Feb 8, 2016

bf4 commented Feb 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

buren commented Feb 11, 2016

bf4 commented Feb 17, 2016

NullVoxPopuli commented Mar 9, 2016