public-activity / public_activity

Easy activity tracking for models - similar to Github's Public Activity
MIT License
2.96k stars 334 forks source link

missing keyword: :coder If no default coder is configured, a coder must be provided to `serialize`. rails 7 #382

Closed santosh-1987 closed 8 months ago

santosh-1987 commented 9 months ago

image

jeffcovey commented 9 months ago

This is holding us back from upgrading to Rails 7.1. Does anyone have a solution?

0q2 commented 9 months ago

How did this pass testing?

MyklClason commented 8 months ago

Docs: https://api.rubyonrails.org/v7.1.2/classes/ActiveRecord/AttributeMethods/Serialization/ClassMethods.html

https://github.com/public-activity/public_activity/blob/5f1c429820723ca03ddacebde707800f8d29309e/lib/public_activity/orm/active_record/activity.rb#L42 Looks like it needs to be changed to something like (default coder was previously YAML)

serialize :parameters, coder: YAML, type: Hash unless %i[json jsonb hstore].include?(columns_hash['parameters'].type)
crustyratfink commented 8 months ago

Running into this. Could use some enlightenment on the cause and any workarounds that might deal with it? Thanks!

MyklClason commented 8 months ago

@crustyratfink PR seems to be in the works (#384), but if I'm reading the code right, there should be a way to add "parameters" as a column type and override this. So perhaps if you include this line of code in the model, it might work. I don't think I actually use this gem (I work on a number of projects with clients, and some of those include Heroku stack upgrades that require resolving issues like this); I just came across it with the same error I was getting, and passed the fix along so others could handle it.

serialize :parameters, coder: YAML, type: Hash unless %i[json jsonb hstore].include?(columns_hash['parameters'].type)
samy-at-shopify commented 8 months ago

+1 to this, its holding us back from bumping to 7.1. The PR @MyklClason linked to seems like a simple and straightforward fix. @ur5us could you please give it a look?

ur5us commented 8 months ago

@0q2

How did this pass testing?

If you take a closer look at https://github.com/public-activity/public_activity#rails-7 you’ll notice the following:

As of version 2.0.0, public_activity also supports Rails up to 7.0.

So in the current release Rails 7.1 is only partially supported in that one has to either monkey patch PublicActivity::Activity or set a default coder. The latter or rather the lack of a default coder config in Rails 7.1 results in the breaking change leading up to this issue here. I’ve fixed the issue weeks ago but haven’t found the time to cut a release yet. I’ll do that shortly.

ur5us commented 8 months ago

@santosh-1987 @jeffcovey @MyklClason @samy-at-shopify I’ve implemented the same fix as provided in https://github.com/public-activity/public_activity/pull/384 weeks ago but initially that broke a few tests and rightly so! As it stands, I’ll have to reject the PR. The issue is that it only works with Rails 7.1+ but our support matrix is much wider than that. Thankfully, it’s straight forward to make it work of < 7.1. I’ll push the code soon and try to cut a release by the end of the week.

In the meantime, you have the following 2 options, though I haven’t tried either of them:

  1. define a default (as in app-wide) coder, i.e. in your application.rb add:
      config.active_record.default_column_serializer = YAML
      # PublicActivity#parameters serialisation
      config.active_record.yaml_column_permitted_classes = […]

    Full details and considerations: https://github.com/rails/rails/blob/v7.1.2/activerecord/CHANGELOG.md?plain=1#L995

  2. monkey patch PublicActivity::Activity in an initializer (e.g. config/initializers/public_activity_defaults.rb) to provide the missing coder: YAML option:

     Rails.application.reloader.to_prepare do
       PublicActivity::Activity.class_eval do
         serialize :parameters,
                          type: Hash,
                          coder: YAML,
                          yaml: { permitted_classes: […] }
       end
     end

    I had to permit the following classes for one of my apps but YMMV:

    • ActiveSupport::TimeWithZone
    • ActiveSupport::TimeZone
    • BigDecimal
    • Date
    • Symbol
    • Time

Alternatively, you can just wait a few more days for the next release as mentioned above.

ur5us commented 8 months ago

@santosh-1987 @jeffcovey @MyklClason @samy-at-shopify I was wrong about the compatibility, s. my comment. @MyklClason you were right.

dorianmariecom commented 1 month ago

For those ending up here that have the same issue with rpush for instance, doing:

config.active_record.default_column_serializer = JSON

in config/application.rb fixes this error