Skip to content

Instantly share code, notes, and snippets.

@jordanbrock
Last active August 7, 2020 02:02
Show Gist options
  • Save jordanbrock/b6fd00c727a2d6a33366554a57e5a7b1 to your computer and use it in GitHub Desktop.
Save jordanbrock/b6fd00c727a2d6a33366554a57e5a7b1 to your computer and use it in GitHub Desktop.
Params deserialisation issue
# Represents a selection of a Player for a Match
class Selection < ApplicationRecord
acts_as_list scope: :match
belongs_to :match
belongs_to :player
# Ensure that the selections are returned in postition order
def self.default_scope
order(position: :asc)
end
end
# To deliver this notification
#
# SelectionNotification.with(post: @post).deliver_later(current_user)
# SelectionNotification.with(post: @post).deliver(current_user)
class SelectionNotification < Noticed::Base
# Add your delivery methods
#
deliver_by :database
# deliver_by :email, mailer: "UserMailer"
# deliver_by :slack
# deliver_by :custom, class: "MyDeliveryMethod"
# Add required params
#
param :selection
# Define helper methods to make rendering easier.
#
# def message
# t(".message")
# end
#
# def url
# post_path(params[:post])
# end
end
@jordanbrock
Copy link
Author

jordanbrock commented Aug 6, 2020

First of all, create the notification

irb(main):005:0> n = SelectionNotification.with(selection: Selection.last)
  Selection Load (0.6ms)  SELECT "selections".* FROM "selections" ORDER BY "selections"."position" DESC LIMIT $1  [["LIMIT", 1]]
=> #<SelectionNotification:0x00007fcc62de5c48 @params={:selection=>#<Selection id: 120, match_id: 19, player_id: 21, captain: 0, wicketkeeper: 0, comment: nil, position: 12, played: 0>}>

Check that the selection is available via params

irb(main):007:0> n.params[:selection]
=> #<Selection id: 120, match_id: 19, player_id: 21, captain: 0, wicketkeeper: 0, comment: nil, position: 12, played: 0>

Then deliver the notification via DB

irb(main):008:0> n.deliver(User.first)
  User Load (0.5ms)  SELECT "users".* FROM "users" ORDER BY "users"."id" ASC LIMIT $1  [["LIMIT", 1]]
Performing Noticed::DeliveryMethods::Database (Job ID: b1ee0032-3e8a-4559-806d-9ede810acc7b) from Async(default) enqueued at  with arguments: {:notification_class=>"SelectionNotification", :options=>{}, :params=>{:selection=>#<GlobalID:0x00007fcc625cbf30 @uri=#<URI::GID gid://coach/Selection/120>>}, :recipient=>#<GlobalID:0x00007fcc625cb5a8 @uri=#<URI::GID gid://coach/User/1>>, :record=>nil}
   (0.3ms)  BEGIN
  Notification Create (1.8ms)  INSERT INTO "notifications" ("recipient_type", "recipient_id", "type", "params", "created_at", "updated_at") VALUES ($1, $2, $3, $4, $5, $6) RETURNING "id"  [["recipient_type", "User"], ["recipient_id", 1], ["type", "SelectionNotification"], ["params", "{\"selection\"=>{\"_aj_globalid\"=>\"gid://coach/Selection/120\"}, \"_aj_symbol_keys\"=>[\"selection\"]}"], ["created_at", "2020-08-06 01:16:23.002529"], ["updated_at", "2020-08-06 01:16:23.002529"]]
   (0.5ms)  COMMIT
Performed Noticed::DeliveryMethods::Database (Job ID: b1ee0032-3e8a-4559-806d-9ede810acc7b) from Async(default) in 5.91ms
=> [#<User id: 1, email: "jordan@brock.id.au", admin: 1, created_at: "2020-05-05 09:31:39", updated_at: "2020-07-17 04:47:12", club_id: 1, player_id: nil, coach: 1, selector: 1, status: 1, firstname: "Jordan", lastname: "Brock", mobile: "+61 416 124 625", approved: true>]

Then load the last notification and attempt to access the selection

irb(main):010:0> n = Notification.last
  Notification Load (0.3ms)  SELECT "notifications".* FROM "notifications" ORDER BY "notifications"."id" DESC LIMIT $1  [["LIMIT", 1]]
=> #<Notification id: 13, recipient_type: "User", recipient_id: 1, type: "SelectionNotification", params: "{\"selection\"=>{\"_aj_globalid\"=>\"gid://coach/Select...", read_at: nil, created_at: "2020-08-06 01:16:23", updated_at: "2020-08-06 01:16:23">

irb(main):011:0> n.to_notification.params[:selection]
Traceback (most recent call last):
        2: from (irb):11
        1: from (irb):11:in `[]'
TypeError (no implicit conversion of Symbol into Integer)

Use :to_notification instead

irb(main):018:0> n.to_notification
=> #<SelectionNotification:0x00007fcc62d681a8 @params="{\"selection\"=>{\"_aj_globalid\"=>\"gid://coach/Selection/120\"}, \"_aj_symbol_keys\"=>[\"selection\"]}", @record=#<Notification id: 13, recipient_type: "User", recipient_id: 1, type: "SelectionNotification", params: "{\"selection\"=>{\"_aj_globalid\"=>\"gid://coach/Select...", read_at: nil, created_at: "2020-08-06 01:16:23", updated_at: "2020-08-06 01:16:23">>

irb(main):019:0> n.to_notification.params[:selection]
Traceback (most recent call last):
        2: from (irb):19
        1: from (irb):19:in `[]'
TypeError (no implicit conversion of Symbol into Integer)

@excid3
Copy link

excid3 commented Aug 6, 2020

What is the output of n.to_notification.params?

@jordanbrock
Copy link
Author

irb(main):020:0> n.to_notification.params
=> "{\"selection\"=>{\"_aj_globalid\"=>\"gid://coach/Selection/120\"}, \"_aj_symbol_keys\"=>[\"selection\"]}"

@excid3
Copy link

excid3 commented Aug 6, 2020

Looks like it's not deserializing at all.

Does your Notification model have the include Noticed::Model in it? That's what sets up the serializer.

@jordanbrock
Copy link
Author

class Notification < ApplicationRecord
  include Noticed::Model

  belongs_to :user, polymorphic: true
end

@excid3
Copy link

excid3 commented Aug 6, 2020

Looks correct. And you're on v1.2.4?

@excid3
Copy link

excid3 commented Aug 6, 2020

And what's the output of n.params?

@jordanbrock
Copy link
Author

Yep

bundle list FYI

Gems included by the bundle:
  * actioncable (6.0.3.1)
  * actionmailbox (6.0.3.1)
  * actionmailer (6.0.3.1)
  * actionpack (6.0.3.1)
  * actiontext (6.0.3.1)
  * actionview (6.0.3.1)
  * activejob (6.0.3.1)
  * activemodel (6.0.3.1)
  * activerecord (6.0.3.1)
  * activestorage (6.0.3.1)
  * activesupport (6.0.3.1)
  * acts_as_list (1.0.1)
  * addressable (2.7.0)
  * ansi (1.5.0)
  * ast (2.4.1)
  * aws-eventstream (1.0.3)
  * aws-partitions (1.240.0)
  * aws-sdk-core (3.77.0)
  * aws-sdk-kms (1.25.0)
  * aws-sdk-s3 (1.54.0)
  * aws-sigv4 (1.1.0)
  * axiom-types (0.1.1)
  * bcrypt (3.1.13)
  * bootsnap (1.4.6)
  * bootstrap-email (0.3.1)
  * brakeman (4.8.2)
  * bugsnag (6.13.1)
  * builder (3.2.4)
  * bundler-audit (0.6.1 31dc79b)
  * byebug (11.1.1)
  * capybara (3.32.0)
  * childprocess (3.0.0)
  * coercible (1.0.0)
  * concurrent-ruby (1.1.6)
  * crack (0.4.3)
  * crass (1.0.6)
  * css_parser (1.7.1)
  * descendants_tracker (0.0.4)
  * devise (4.7.1)
  * docile (1.3.2)
  * domain_name (0.5.20190701)
  * dotenv (2.7.5)
  * dotenv-rails (2.7.5)
  * equalizer (0.0.11)
  * erubi (1.9.0)
  * erubis (2.7.0)
  * ffi (1.13.1)
  * ffi-compiler (1.0.1)
  * flay (2.12.1)
  * flog (4.6.4)
  * globalid (0.4.2)
  * groupdate (5.0.0)
  * haml (5.1.2)
  * haml-rails (2.0.1)
  * haml_lint (0.35.0)
  * hashdiff (1.0.0)
  * html2haml (2.2.0)
  * htmlentities (4.3.4)
  * http (4.4.1)
  * http-cookie (1.0.3)
  * http-form_data (2.3.0)
  * http-parser (1.2.1)
  * i18n (1.8.5)
  * ice_nine (0.11.2)
  * image_processing (1.9.0)
  * jbuilder (2.10.0)
  * jmespath (1.4.0)
  * kwalify (0.7.2)
  * launchy (2.5.0)
  * listen (3.1.5)
  * loofah (2.6.0)
  * mail (2.7.1)
  * marcel (0.3.3)
  * meta_request (0.7.2)
  * metaclass (0.0.4)
  * method_source (1.0.0)
  * mimemagic (0.3.5)
  * mini_magick (4.9.5)
  * mini_mime (1.0.2)
  * mini_portile2 (2.4.0)
  * minitest (5.14.1)
  * minitest-reporters (1.4.2)
  * mocha (1.9.0)
  * msgpack (1.3.3)
  * nio4r (2.5.2)
  * nokogiri (1.10.10)
  * noticed (1.2.4)
  * orm_adapter (0.5.0)
  * parallel (1.19.1)
  * parser (2.7.1.3)
  * path_expander (1.1.0)
  * pg (1.2.3)
  * polyglot (0.3.5)
  * premailer (1.11.1)
  * premailer-rails (1.11.1)
  * prettier (0.18.2)
  * psych (3.1.0)
  * public_suffix (4.0.5)
  * puma (4.3.3)
  * pundit (2.1.0)
  * rack (2.2.3)
  * rack-contrib (2.1.0)
  * rack-proxy (0.6.5)
  * rack-test (1.1.0)
  * rails (6.0.3.1)
  * rails-dom-testing (2.0.3)
  * rails-html-sanitizer (1.3.0)
  * railties (6.0.3.1)
  * rainbow (3.0.0)
  * rake (13.0.1)
  * rb-fsevent (0.10.3)
  * rb-inotify (0.10.1)
  * reek (6.0.1)
  * regexp_parser (1.7.1)
  * responders (3.0.1)
  * rexml (3.2.4)
  * rubocop (0.85.1)
  * rubocop-ast (0.0.3)
  * rubocop-rails (2.6.0 6c9224d)
  * ruby-progressbar (1.10.1)
  * ruby-vips (2.0.14)
  * ruby_dep (1.5.0)
  * ruby_parser (3.14.2)
  * rubycritic (4.5.0)
  * rubyzip (2.3.0)
  * safe_yaml (1.0.5)
  * sass-rails (6.0.0)
  * sassc (2.4.0)
  * sassc-rails (2.1.2)
  * search_cop (1.1.0)
  * selenium-webdriver (3.142.7)
  * sexp_processor (4.14.1)
  * shopify-money (0.11.5)
  * simple_calendar (2.3.0)
  * simple_form (5.0.2)
  * simplecov (0.18.5)
  * simplecov-html (0.12.2)
  * spring (2.1.0)
  * sprockets (4.0.2)
  * sprockets-rails (3.2.1)
  * sysexits (1.2.0)
  * temple (0.8.2)
  * thor (1.0.1)
  * thread_safe (0.3.6)
  * tilt (2.0.10)
  * treetop (1.6.10)
  * tty-which (0.4.2)
  * turbolinks (5.2.1)
  * turbolinks-source (5.2.0)
  * tzinfo (1.2.7)
  * unf (0.1.4)
  * unf_ext (0.0.7.7)
  * unicode-display_width (1.7.0)
  * view_component (2.7.0)
  * virtus (1.0.5)
  * warden (1.2.8)
  * webdrivers (4.4.1)
  * webmock (3.7.6)
  * webpacker (4.2.2)
  * websocket-driver (0.7.3)
  * websocket-extensions (0.1.5)
  * xpath (3.2.0)
  * zeitwerk (2.4.0)

@jordanbrock
Copy link
Author

irb(main):002:0> n.params
=> "{\"selection\"=>{\"_aj_globalid\"=>\"gid://coach/Selection/120\"}, \"_aj_symbol_keys\"=>[\"selection\"]}"

@excid3
Copy link

excid3 commented Aug 6, 2020

Okay, definitely not serializing which is very strange. Gonna try and replicate in a new Rails 6 app.

@jordanbrock
Copy link
Author

Me too. Thanks.

@excid3
Copy link

excid3 commented Aug 6, 2020

I was able to reproduce it. Not sure what's causing it, but at least I can find out now. 👍

@jordanbrock
Copy link
Author

well, good news I guess. Thanks for your help. At least I wasn't going crazy.

@excid3
Copy link

excid3 commented Aug 6, 2020

Yeah, strange cuz it's working in Jumpstart Pro and the tests for Noticed. o_O

@excid3
Copy link

excid3 commented Aug 6, 2020

And it's clearly serializing, just not deserializing properly.

@excid3
Copy link

excid3 commented Aug 6, 2020

Ah, you know this is because of using the text column instead of json I bet.

I was testing a lot with json columns and then switched to a text column for the generator recently so it'd be more compatible. It's giving a string back, so we probably need to parse that back into a hash before sending it into ActiveJob's argument deserializer.

@excid3
Copy link

excid3 commented Aug 6, 2020

Yep, just confirmed that. We can work on a solution for that tomorrow. I'm not sure what the best way to handle it will be, but we can figure that out.

@jordanbrock
Copy link
Author

Clunky, but probably better than the alternative on having to support text on things like AuroraMySQL, but then json on vanilla mysql and Postgres.

@jordanbrock
Copy link
Author

👍

@excid3
Copy link

excid3 commented Aug 6, 2020

Yeah, I mean I want to support sqlite and MySQL so we definitely need support for text columns.

Maybe we can detect the column type or we can make it a config option. I'm not real sure.

@excid3
Copy link

excid3 commented Aug 6, 2020

Fixed! Thanks for the heads up on this 🙏

As it turns out, my test suite was still using a jsonb column and I thought I had changed that. I've added tests for text, json and jsonb column types now so we can know it'll work for them all now.

@excid3
Copy link

excid3 commented Aug 6, 2020

And in case you're interested, the solution was to serialize with ActiveJob's serializer, then serialize with JSON. Reverse on the way out.

Previously it was writing the string representation of the hash to the text column. We don't want to eval that to load it back out, so converting to JSON was safer and more compatible.

@jordanbrock
Copy link
Author

Awesome stuff Chris. Trying it out now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment