From 663690f3bcd44ca30ca904533f43544f3b41df1d Mon Sep 17 00:00:00 2001 From: Chris Oliver Date: Tue, 23 Jan 2024 08:56:04 -0600 Subject: [PATCH 01/10] Validate params should check presence of key, not value --- CHANGELOG.md | 2 ++ app/models/concerns/noticed/deliverable.rb | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ade56a63..7005c0d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ### Unreleased +* Validate param key exists, not the value + ### 2.0.3 * Notifier generator now ensures the `Notifier` suffix. diff --git a/app/models/concerns/noticed/deliverable.rb b/app/models/concerns/noticed/deliverable.rb index e0a385ba..05621c07 100644 --- a/app/models/concerns/noticed/deliverable.rb +++ b/app/models/concerns/noticed/deliverable.rb @@ -103,7 +103,7 @@ def validate! def validate_params! required_param_names.each do |param_name| - raise ValidationError, "Param `#{param_name}` is required for #{self.class.name}." unless params[param_name].present? + raise ValidationError, "Param `#{param_name}` is required for #{self.class.name}." unless params.has_key?(param_name) end end From 395db0aa19f1e7db42ab5cebd7ac5ef02528441f Mon Sep 17 00:00:00 2001 From: Chris Oliver Date: Tue, 23 Jan 2024 08:56:40 -0600 Subject: [PATCH 02/10] Use Noticed::Notifications channel in dummy app --- test/delivery_methods/action_cable_test.rb | 4 ++-- test/dummy/app/channels/notification_channel.rb | 9 --------- 2 files changed, 2 insertions(+), 11 deletions(-) delete mode 100644 test/dummy/app/channels/notification_channel.rb diff --git a/test/delivery_methods/action_cable_test.rb b/test/delivery_methods/action_cable_test.rb index 38c8b163..94551265 100644 --- a/test/delivery_methods/action_cable_test.rb +++ b/test/delivery_methods/action_cable_test.rb @@ -9,10 +9,10 @@ class ActionCableDeliveryMethodTest < ActiveSupport::TestCase test "sends websocket message" do user = users(:one) - channel = NotificationChannel.broadcasting_for(user) + channel = Noticed::NotificationChannel.broadcasting_for(user) set_config( - channel: "NotificationChannel", + channel: "Noticed::NotificationChannel", stream: user, message: {foo: :bar} ) diff --git a/test/dummy/app/channels/notification_channel.rb b/test/dummy/app/channels/notification_channel.rb deleted file mode 100644 index 66cdb454..00000000 --- a/test/dummy/app/channels/notification_channel.rb +++ /dev/null @@ -1,9 +0,0 @@ -class NotificationChannel < ApplicationCable::Channel - def subscribed - stream_for current_user - end - - def unsubscribed - stop_all_streams - end -end From d24d6553e6b5f7a8b306bf22743a0cdfaac96188 Mon Sep 17 00:00:00 2001 From: Phil Reynolds Date: Tue, 23 Jan 2024 17:02:05 +0000 Subject: [PATCH 03/10] Add {message: {}} wrapper and format_notification method --- lib/noticed/delivery_methods/fcm.rb | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/noticed/delivery_methods/fcm.rb b/lib/noticed/delivery_methods/fcm.rb index ad166637..1b127dea 100644 --- a/lib/noticed/delivery_methods/fcm.rb +++ b/lib/noticed/delivery_methods/fcm.rb @@ -14,7 +14,7 @@ def deliver def send_notification(device_token) post_request("https://fcm.googleapis.com/v1/projects/#{credentials[:project_id]}/messages:send", headers: {authorization: "Bearer #{access_token}"}, - json: notification.instance_exec(device_token, &config[:json])) + json: {message: format_notification(device_token)}) rescue Noticed::ResponseUnsuccessful => exception if exception.response.code == "404" && config[:invalid_token] notification.instance_exec(device_token, &config[:invalid_token]) @@ -23,6 +23,16 @@ def send_notification(device_token) end end + def format_notification(device_token) + raise ArgumentError, "No json for fcm delivery. Add the 'json' option in 'deliver_by :fcm'." unless (method = config[:json]) + + if method.is_a?(Symbol) && event.respond_to?(method) + event.send(method, device_token) + else + notification.instance_exec(device_token, &method) + end + end + def credentials @credentials ||= begin value = evaluate_option(:credentials) From eccaa5dec417499843eb2ebe96a851bc3a7a0fb4 Mon Sep 17 00:00:00 2001 From: Phil Reynolds Date: Tue, 23 Jan 2024 17:04:22 +0000 Subject: [PATCH 04/10] update test to remove requirement for message:{} wrapper, it now matches the examples in the documentation --- test/delivery_methods/fcm_test.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/delivery_methods/fcm_test.rb b/test/delivery_methods/fcm_test.rb index 3eb58275..4f1c6445 100644 --- a/test/delivery_methods/fcm_test.rb +++ b/test/delivery_methods/fcm_test.rb @@ -26,10 +26,8 @@ def fetch_access_token! device_tokens: [:a, :b], json: ->(device_token) { { - message: { - token: device_token, - notification: {title: "Title", body: "Body"} - } + token: device_token, + notification: {title: "Title", body: "Body"} } } ) From 8f17119d3be3f546c0158778c51ef764f1aa8c4f Mon Sep 17 00:00:00 2001 From: Phil Reynolds Date: Tue, 23 Jan 2024 17:21:38 +0000 Subject: [PATCH 05/10] update FCM docs with v2.0 examples and extra information --- docs/delivery_methods/fcm.md | 91 ++++++++++++++++++++++++++---------- 1 file changed, 66 insertions(+), 25 deletions(-) diff --git a/docs/delivery_methods/fcm.md b/docs/delivery_methods/fcm.md index 759a1bdd..e2eff32b 100644 --- a/docs/delivery_methods/fcm.md +++ b/docs/delivery_methods/fcm.md @@ -26,6 +26,7 @@ See the below instructions on where to store this information within your applic class CommentNotification deliver_by :fcm do |config| config.credentials = Rails.root.join("config/certs/fcm.json") + config.device_tokens = ->(recipient) { recipient.notification_tokens.where(platform: "fcm").pluck(:token) } config.json = ->(device_token) { { token: device_token, @@ -35,42 +36,82 @@ class CommentNotification } } } + config.if = ->(recipient) { recipient.android_notifications? } end end ``` ## Options -* `json` - Customize the Firebase Cloud Messaging notification object. This can be a Lambda or Symbol of a method name on the notifier. The callable object will be given the device token as an argument. +### `json` +Customize the Firebase Cloud Messaging notification object. This can be a Lambda or Symbol of a method name on the notifier. + +The callable object will be given the device token as an argument. -* `credentials` +We wrap the object with the required `{message: {}}` structure. + +There are lots of options of now to structure a FCM notification message. See https://firebase.google.com/docs/cloud-messaging/concept-options for more details. + +### `credentials` The location of your Firebase Cloud Messaging credentials. -- When a String object: `deliver_by :fcm, credentials: "config/credentials/fcm.json"` - * Interally, this string is passed to `Rails.root.join()` as an argument so there is no need to do this beforehand. -- When a Pathname object: `deliver_by :fcm, credentials: Rails.root.join("config/credentials/fcm.json")` - * The Pathname object can point to any location where you are storing your credentials. -- When a Hash object: `deliver_by :fcm, credentials: credentials_hash` - ```ruby - credentials_hash = { - "type": "service_account", - "project_id": "test-project-1234", - "private_key_id": ..., - etc..... - } - ``` - * A Hash which contains your credentials -- When a Symbol: `deliver_by :fcm, credentials: :fcm_credentials, format: :format_notification` - ```ruby - def fcm_credentials - Rails.root.join("config/certs/fcm.json") - end - ``` +#### When a String Object + +Internally, this string is passed to `Rails.root.join()` as an argument so there is no need to do this beforehand. + +```ruby +deliver_by :fcm do |config| + config.credentials = "config/credentials/fcm.json" +end +``` + +#### When a Pathname object + +The Pathname object can point to any location where you are storing your credentials. + +```ruby +deliver_by :fcm do |config| + config.credentials = Rails.root.join("config/credentials/fcm.json") +end +``` + +#### When a Hash object + +A Hash which contains your credentials + +```ruby +deliver_by :fcm do |config| + config.credentials = credentials_hash +end + +credentials_hash = { + "type": "service_account", + "project_id": "test-project-1234", + "private_key_id": ..., + etc..... +} +``` + +#### When a Symbol + +Points to a method which can return a Hash of your credentials, Pathname, or String to your credentials like the examples above. + +We pass the notification object as an argument to the method. If you don't need to use it you can use the splat operator `(*)` to ignore it. + +```ruby +deliver_by :fcm do |config| + config.credentials = :fcm_credentials + config.json = :format_notification +end + +def fcm_credentials(*) + Rails.root.join("config/certs/fcm.json") +end +``` - * Points to a method which can return a Hash of your credentials, Pathname, or String to your credentials like the examples above. +#### Otherwise -Otherwise, if the credentials option is left out, it will look for your credentials in: `Rails.application.credentials.fcm` +If the credentials option is left out, it will look for your credentials in: `Rails.application.credentials.fcm` ## Gathering Notification Tokens From 87ad883a45801d2e689a5bfc2c813e3e615173b2 Mon Sep 17 00:00:00 2001 From: Alex VanRielly Date: Tue, 23 Jan 2024 11:04:58 -0700 Subject: [PATCH 06/10] Adding back in the parent class override for Noticed::ActiveJob --- README.md | 9 ++++++++- lib/noticed.rb | 4 ++++ lib/noticed/delivery_method.rb | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 676b84a5..e0288596 100644 --- a/README.md +++ b/README.md @@ -487,6 +487,14 @@ end # @post.noticed_events.each { |ne| ne.notifications... } ``` +#### ActiveJob Parent Class + +Noticed uses its own `Noticed::ApplicationJob` as the base job for all notifications. In the event that you would like to customize the parent job class, there is a `parent_class` attribute that can be overridden with your own class. This should be done in a `noticed.rb` initializer. + +```ruby +Noticed.parent_class = "ApplicationJob" +``` + #### Handling Deleted Records Generally we recommend using a `dependent: ___` relationship on your models to avoid cases where Noticed Events or Notifications are left lingering when your models are destroyed. In the case that they are or data becomes mis-matched, you’ll likely run into deserialization issues. That may be globally alleviated with the following snippet, but use with caution. @@ -511,4 +519,3 @@ DATABASE_URL=postgres://127.0.0.1/noticed_test rails test ## 📝 License The gem is available as open source under the terms of the [MIT License](https://opensource.org/licenses/MIT). - diff --git a/lib/noticed.rb b/lib/noticed.rb index c39d548d..a95aa63a 100644 --- a/lib/noticed.rb +++ b/lib/noticed.rb @@ -39,6 +39,10 @@ module DeliveryMethods autoload :Webhook, "noticed/delivery_methods/webhook" end + mattr_accessor :parent_class + @@parent_class = "Noticed::ApplicationJob" + + class ValidationError < StandardError end diff --git a/lib/noticed/delivery_method.rb b/lib/noticed/delivery_method.rb index f84dc63e..5e7b7069 100644 --- a/lib/noticed/delivery_method.rb +++ b/lib/noticed/delivery_method.rb @@ -1,5 +1,5 @@ module Noticed - class DeliveryMethod < ApplicationJob + class DeliveryMethod < Noticed.parent_class.constantize include ApiClient include RequiredOptions From 4b7de15af6de86e2474bcbd8055aafb7af46bd79 Mon Sep 17 00:00:00 2001 From: Alex VanRielly Date: Tue, 23 Jan 2024 11:21:59 -0700 Subject: [PATCH 07/10] Fixing standardrb issue --- lib/noticed.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/noticed.rb b/lib/noticed.rb index a95aa63a..444a34c2 100644 --- a/lib/noticed.rb +++ b/lib/noticed.rb @@ -42,7 +42,6 @@ module DeliveryMethods mattr_accessor :parent_class @@parent_class = "Noticed::ApplicationJob" - class ValidationError < StandardError end From 7c4a7320dfd9f489ced7e3ce54a77f56d4213c2f Mon Sep 17 00:00:00 2001 From: Phil Reynolds Date: Tue, 23 Jan 2024 19:11:17 +0000 Subject: [PATCH 08/10] Remove 'message{}' wrapping from JSON payload in fcm.rb --- UPGRADE.md | 35 +++++++++++++++-------------- docs/delivery_methods/fcm.md | 12 +++++----- lib/noticed/delivery_methods/fcm.rb | 2 +- test/delivery_methods/fcm_test.rb | 9 ++++---- 4 files changed, 30 insertions(+), 28 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index cec37485..bd0eb17f 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -215,41 +215,42 @@ Options for delivery methods have been renamed for clarity and consistency. #### ActionCable -The `format` option has been renamed to `message`. -The `Noticed::NotificationChannel` has been removed and an example channel is provided in the [ActionCable docs](docs/delivery_methods/action_cable.md). +- The `format` option has been renamed to `message`. +- The `Noticed::NotificationChannel` has been removed and an example channel is provided in the [ActionCable docs](docs/delivery_methods/action_cable.md). #### Email Delivery Method -`method` is now a required option. Previously, it was inferred from the notification name but we've decided it would be better to be explicit. +- `method` is now a required option. Previously, it was inferred from the notification name but we've decided it would be better to be explicit. #### FCM -The `format` option has been renamed to `json`. -The `device_tokens` option is now required and should return an Array of device tokens. -The `invalid_token` option replaces the `cleanup_device_tokens` method for handling invalid/expired tokens. +- The `format` option has been renamed to `json`. +- The `device_tokens` option is now required and should return an Array of device tokens. +- The `invalid_token` option replaces the `cleanup_device_tokens` method for handling invalid/expired tokens. +- We no longer wrap the json payload in the `message{}` key. This means we are more compatible with the FCM docs and any future changes that Google make. #### iOS -The `cert_path` option has been renamed to `apns_key` and should be given the key and not a path. -The `format` option has been renamed to `json`. -The `device_tokens` option is now required and should return an Array of device tokens. -The `invalid_token` option replaces the `cleanup_device_tokens` method for handling invalid/expired tokens. +- The `cert_path` option has been renamed to `apns_key` and should be given the key and not a path. +- The `format` option has been renamed to `json`. +- The `device_tokens` option is now required and should return an Array of device tokens. +- The `invalid_token` option replaces the `cleanup_device_tokens` method for handling invalid/expired tokens. #### Microsoft Teams -The `format` option has been renamed to `json`. +- The `format` option has been renamed to `json`. #### Slack -The `format` option has been renamed to `json`. -The `url` option now defaults to `"https://slack.com/api/chat.postMessage` instead of `Rails.application.credentials.dig(:slack, :notification_url)` +- The `format` option has been renamed to `json`. +- The `url` option now defaults to `"https://slack.com/api/chat.postMessage` instead of `Rails.application.credentials.dig(:slack, :notification_url)` #### Twilio Messaging -Twilio has been renamed to `:twilio_messaging` to make room for `:twilio_voice` and other services they may provide in the future. -The `format` option has been renamed to `json`. +- Twilio has been renamed to `:twilio_messaging` to make room for `:twilio_voice` and other services they may provide in the future. +- The `format` option has been renamed to `json`. #### Vonage SMS -Vonage has been renamed to `:vonage_sms` to make room for other Vonage services in the future. -The `format` option has been renamed to `json` and is now required. +- Vonage has been renamed to `:vonage_sms` to make room for other Vonage services in the future. +- The `format` option has been renamed to `json` and is now required. diff --git a/docs/delivery_methods/fcm.md b/docs/delivery_methods/fcm.md index e2eff32b..09aa5af5 100644 --- a/docs/delivery_methods/fcm.md +++ b/docs/delivery_methods/fcm.md @@ -29,10 +29,12 @@ class CommentNotification config.device_tokens = ->(recipient) { recipient.notification_tokens.where(platform: "fcm").pluck(:token) } config.json = ->(device_token) { { - token: device_token, - notification: { - title: "Test Title", - body: "Test body" + message: { + token: device_token, + notification: { + title: "Test Title", + body: "Test body" + } } } } @@ -48,8 +50,6 @@ Customize the Firebase Cloud Messaging notification object. This can be a Lambda The callable object will be given the device token as an argument. -We wrap the object with the required `{message: {}}` structure. - There are lots of options of now to structure a FCM notification message. See https://firebase.google.com/docs/cloud-messaging/concept-options for more details. ### `credentials` diff --git a/lib/noticed/delivery_methods/fcm.rb b/lib/noticed/delivery_methods/fcm.rb index 1b127dea..49431324 100644 --- a/lib/noticed/delivery_methods/fcm.rb +++ b/lib/noticed/delivery_methods/fcm.rb @@ -14,7 +14,7 @@ def deliver def send_notification(device_token) post_request("https://fcm.googleapis.com/v1/projects/#{credentials[:project_id]}/messages:send", headers: {authorization: "Bearer #{access_token}"}, - json: {message: format_notification(device_token)}) + json: format_notification(device_token)) rescue Noticed::ResponseUnsuccessful => exception if exception.response.code == "404" && config[:invalid_token] notification.instance_exec(device_token, &config[:invalid_token]) diff --git a/test/delivery_methods/fcm_test.rb b/test/delivery_methods/fcm_test.rb index 4f1c6445..1467f262 100644 --- a/test/delivery_methods/fcm_test.rb +++ b/test/delivery_methods/fcm_test.rb @@ -25,10 +25,11 @@ def fetch_access_token! }, device_tokens: [:a, :b], json: ->(device_token) { - { - token: device_token, - notification: {title: "Title", body: "Body"} - } + {message: + { + token: device_token, + notification: {title: "Title", body: "Body"} + }} } ) From 8988b2c4aad0187f908492273d7e10d8eb459afb Mon Sep 17 00:00:00 2001 From: Phil Reynolds Date: Tue, 23 Jan 2024 19:58:06 +0000 Subject: [PATCH 09/10] remove unnecessary raise as json is required for fcm --- lib/noticed/delivery_methods/fcm.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/noticed/delivery_methods/fcm.rb b/lib/noticed/delivery_methods/fcm.rb index 49431324..99ad60c1 100644 --- a/lib/noticed/delivery_methods/fcm.rb +++ b/lib/noticed/delivery_methods/fcm.rb @@ -24,8 +24,7 @@ def send_notification(device_token) end def format_notification(device_token) - raise ArgumentError, "No json for fcm delivery. Add the 'json' option in 'deliver_by :fcm'." unless (method = config[:json]) - + method = config[:json] if method.is_a?(Symbol) && event.respond_to?(method) event.send(method, device_token) else From ab58bf86ea73f8bb86d55f0f53f0e8fa1461d810 Mon Sep 17 00:00:00 2001 From: Chris Oliver Date: Tue, 23 Jan 2024 14:31:02 -0600 Subject: [PATCH 10/10] Update fcm_test.rb --- test/delivery_methods/fcm_test.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/delivery_methods/fcm_test.rb b/test/delivery_methods/fcm_test.rb index 1467f262..3eb58275 100644 --- a/test/delivery_methods/fcm_test.rb +++ b/test/delivery_methods/fcm_test.rb @@ -25,11 +25,12 @@ def fetch_access_token! }, device_tokens: [:a, :b], json: ->(device_token) { - {message: - { + { + message: { token: device_token, notification: {title: "Title", body: "Body"} - }} + } + } } )