enable key_transform option outside of controller

[jachenry]

ActiveModel::SerializableResource.new(resource).serializable_hash

Expected behavior vs actual behavior

Expected: keys should be transformed using ActiveModelSerializers.config.key_transform
Actual: keys are not transformed

Steps to reproduce

1. add ActiveModelSerializers.config.key_transform = :camel_lower to initializer
2. open rails console
3. run ActiveModel::SerializableResource.new(resource).serializable_hash

Environment

ActiveModelSerializers Version: 61412d8
Ruby version: ruby 2.2.2p95 (2015-04-13 revision 50295) [x86_64-darwin14]
OS Version: Mac OS X El Capitan (10.11.3)
Integrated application: Rails

[groyoh]

Thanks for the clear issue report. #1572 should fix this. Could you try it out with this branch maybe and let us know if it works for you?

[jachenry]

@groyoh I'm seeing the same results with the PR branch. In my case I'm setting the ActiveModelSerializers.config.key_transform option to :camel_lower in the initializer. I would expect this configuration value to return keys with lower camel (created_at -> createdAt) even when running commands like ActiveModel::SerializableResource.new(resource).serializable_hash outside the controller.

[groyoh]

My bad. It seems like the transform_key_casing! (https://github.com/rails-api/active_model_serializers/blob/master/lib/active_model_serializers/adapter/base.rb#L73) method expect the options to contain a serialization context to actually transform the keys. You are indeed right, this seems more like a bug. You should be able to transform the keys without passing the serialization_context.

Seems like a fix would be:

diff --git a/lib/active_model_serializers/adapter/base.rb b/lib/active_model_serializers/adapter/base.rb
index cc6092d..6fe9121 100644
--- a/lib/active_model_serializers/adapter/base.rb
+++ b/lib/active_model_serializers/adapter/base.rb
@@ -64,13 +64,12 @@ module ActiveModelSerializers
       # @param serialization_context [Object] the SerializationContext
       # @return [Symbol] the transform to use
       def key_transform(serialization_context)
-        serialization_context.key_transform ||
+        serialization_context && serialization_context.key_transform ||
         ActiveModelSerializers.config.key_transform ||
         default_key_transform
       end

       def transform_key_casing!(value, serialization_context)
-        return value unless serialization_context
         transform = key_transform(serialization_context)
         KeyTransform.send(transform, value)
       end

@jachenry would you like to provide a PR for fixing this? 😄

@remear what was the idea behind putting the option into the serialization_context? For me, this belongs to the adapter options and not the serialization_context (ref. https://github.com/rails-api/active_model_serializers/pull/1574/files#diff-426cb6b4484fa95a18649ea3cc74474aR72).

[remear]

The goal was to allow specifying a key transform on a per-request basis. If an adapter option can achieve the same thing, then I have no specific objection to it. However, this is probably a better question for @bf4 as many older issues regarding key transformation mention that it should be implemented in the serialization_context.

[groyoh]

It would indeed achieve a per-request transformation. Actually, changing it to an adapter option wouldn't even change the interface when used within a controller e.g. render json: resource, key_transform: :camel. The main advantage would be to be able to use it without a serialization_context e.g. using it outside a controller.

[jachenry]

@groyoh would you still like me to issue a PR or are we changing to an adapter option?

[groyoh]

@jachenry If this is blocking you in any way, please provide a PR and I'll be happy to merge it 😉

In the meantime, @bf4 could you give your point of view on the :key_format being an serialization_context option?

[bf4]

The goal was to allow specifying a key transform on a per-request basis. If an adapter option can achieve the same thing, then I have no specific objection to it.

Sounds good

:key_format being an serialization_context option?

sure, let it go straight to the adapter

[jachenry]

@remear I believe your PR solved this issue so I am going to close. Please let me know if there is a reason to reopen.