symbolize attachment keys

[shanehofstetter]

When passing ActionController::Parameters to the model with the attached base64 file the hash keys are strings, which seems currently not to be expected.

Example usage:

# user.rb
class User < ApplicationRecord
  include ActiveStorageSupport::SupportForBase64

  has_one_base64_attached :avatar
end

# users_controller.rb
def create
  @user = User.new(user_params)
  @user.save
end

def user_params
  params.require(:user).permit(:email, :password, :password_confirmation, avatar: [:data])
end

[santib]

Hey @shanehofstetter , thanks for opening the PR.

We have had this discussion before, when developing the gem and I have some opinions against symbolizing the keys of the hash in our gem. Let me explain myself below with a concrete example:

1. Someone installs `active-storage-base64` gem.
2. Makes use of `io` option (instead of base64 data) in some place using it this way: `{'io' => [stringio], 'filename' => 'something' }` (notice 'io' is a string instead of a symbol)
3. It removes the `active-storage-base64` gem because he/she doesn't need it anymore.

Expected result:
Everything continues working (particularly in the cases he/she was using `io` option for attaching things)

Actual result:
Everything stops working because he/she is sending the keys as strings but ActiveStorage uses keyword arguments in their implementation: https://github.com/rails/rails/blob/e5f4162b6178489181e3d7e7163ac12b7e0efc9d/activestorage/app/models/active_storage/blob.rb#L60

I'm really concerned about breaking people's project just because we decided to symbolize things inside our gem which allows things that Rails itself doesn't (sending 'io' as a string for example). Was I clear? What are your thoughts on this?

[shanehofstetter]

Hi @santib, I completely see your point, I wasn't sure active storage itself didn't support stringified keys (yet?).

My opinion is that without this fix, it's a bit cumbersome to use since you'd need to either symbolize the keys in the controller or attach the data explicitly (which seems not to be necessary when using active-storage without this gem).

Another idea would be to not rely on the parameter being a hash, but instead support passing the base64-data directly as a string (like this for example: https://github.com/rails/rails/blob/5-2-stable/activestorage/lib/active_storage/attached.rb#L18).

[santib]

@shanehofstetter Exactly, I thought symbolizing keys in the controller was the right solution. Do you think explicitly saying in the README to symbolize keys in the controller could help?

Even if I prefer the above mentioned solution, I'm open to change things to make it easier for users, in the end that's what gems are for. Regarding sending just a string with the bas64 data to attach I remember in the past having the discussion with @Ricoch and saying I didn't like it, but now I'm open to change my mind because I don't see any other better solution.

@shanehofstetter if @Ricoch agrees going with the attach([base64 string]) solution, I'm good too.

[Ricoch]

Hi everyone, I am willing to go back to supporting attach([base64 string]) syntax. Will get working on it as soon as I can. I could also add the suggestion of symbolizing the keys on the controller to the Readme file if it makes the gem easier to use for everyone

[Ricoch]

Don't think I will have much time anywhere soon, so, if you are willing we are glad to accept a PR from you so it maybe it's faster

[santib]

@shanehofstetter @Ricoch I just found out that the current implementation of the gem doesn't let you attach images when sent as nested params (even if you symbolize keys in the controller as I suggested previously) because the rails internal logic of nested params sends to the attach method a HashWithIndifferentAccess instead of a plain Hash .

If you clean up the commits (there are 2 unnecessary commits) I'm ok to merge this PR as it is given it will add support that we are missing.

[shanehofstetter]

@santib Nice find! I cleaned up the commits.