From 56cd86fb2b4eb4978dc7ba871ee9b11fb07caf9e Mon Sep 17 00:00:00 2001 From: milos-dukic Date: Fri, 7 Nov 2025 18:38:04 +0100 Subject: [PATCH] [Bugfix] - invoking install generator fixed. Controllers and models for engine renamed. Fixed params passing for creating the subscriptions in controller. Namespace for engine added due to name collision. Web_push success check fixed, tests updated. Readme updated - Frontend Integration fixed. Some new lines and spaces fixed. --- README.md | 24 +++++++++++++------ .../subscriptions_controller.rb | 8 ++++--- .../subscription.rb | 0 config/routes.rb | 4 ++-- .../delivery_methods/web_push.rb | 6 ++--- lib/actionwebpush/instrumentation.rb | 2 +- lib/actionwebpush/notification.rb | 4 ++-- .../campfire_migration_generator.rb | 2 +- .../action_web_push}/install_generator.rb | 17 +++++++++---- .../templates/campfire_compatibility.rb | 0 .../templates/campfire_data_migration.rb | 0 .../create_action_web_push_subscriptions.rb | 2 +- .../action_web_push}/templates/initializer.rb | 0 .../action_web_push}/vapid_keys_generator.rb | 0 test/comprehensive_test.rb | 8 +++---- test/delivery_methods_test.rb | 12 +++++----- 16 files changed, 54 insertions(+), 35 deletions(-) rename app/controllers/{actionwebpush => action_web_push}/subscriptions_controller.rb (86%) rename app/models/{actionwebpush => action_web_push}/subscription.rb (100%) rename lib/{actionwebpush/generators => generators/action_web_push}/campfire_migration_generator.rb (99%) rename lib/{actionwebpush/generators => generators/action_web_push}/install_generator.rb (76%) rename lib/{actionwebpush/generators => generators/action_web_push}/templates/campfire_compatibility.rb (100%) rename lib/{actionwebpush/generators => generators/action_web_push}/templates/campfire_data_migration.rb (100%) rename lib/{actionwebpush/generators => generators/action_web_push}/templates/create_action_web_push_subscriptions.rb (99%) rename lib/{actionwebpush/generators => generators/action_web_push}/templates/initializer.rb (100%) rename lib/{actionwebpush/generators => generators/action_web_push}/vapid_keys_generator.rb (100%) diff --git a/README.md b/README.md index 0626206..86debfe 100644 --- a/README.md +++ b/README.md @@ -320,27 +320,37 @@ ActionWebPush::Subscription.expired.destroy_all Example JavaScript for subscription management: ```javascript +// This is only one implementation of this function, so you can override it if you wish +function arrayBufferToBase64( buffer ) { + var binary = ''; + var bytes = new Uint8Array( buffer ); + var len = bytes.byteLength; + for (var i = 0; i < len; i++) { + binary += String.fromCharCode( bytes[ i ] ); + } + return window.btoa( binary ); +} + // Register service worker and get subscription -navigator.serviceWorker.register('/sw.js').then(registration => { +navigator.serviceWorker.register('/service-worker.js').then(registration => { return registration.pushManager.subscribe({ userVisibleOnly: true, - applicationServerKey: '<%= ActionWebPush.config.vapid_public_key %>' + applicationServerKey: new Uint8Array(<%= Base64.urlsafe_decode64(ActionWebPush.config.vapid_public_key).bytes %>) }); }).then(subscription => { // Send subscription to your Rails app - fetch('/push_subscriptions', { + fetch('/awp/push/subscriptions', { method: 'POST', headers: { 'Content-Type': 'application/json', 'X-CSRF-Token': document.querySelector('[name="csrf-token"]').content }, body: JSON.stringify({ + user_id: // pass user id here, subscription: { endpoint: subscription.endpoint, - keys: { - p256dh: arrayBufferToBase64(subscription.getKey('p256dh')), - auth: arrayBufferToBase64(subscription.getKey('auth')) - } + p256dh_key: arrayBufferToBase64(subscription.getKey('p256dh')), + auth_key: arrayBufferToBase64(subscription.getKey('auth')) } }) }); diff --git a/app/controllers/actionwebpush/subscriptions_controller.rb b/app/controllers/action_web_push/subscriptions_controller.rb similarity index 86% rename from app/controllers/actionwebpush/subscriptions_controller.rb rename to app/controllers/action_web_push/subscriptions_controller.rb index 36bdbd9..3e53859 100644 --- a/app/controllers/actionwebpush/subscriptions_controller.rb +++ b/app/controllers/action_web_push/subscriptions_controller.rb @@ -4,7 +4,7 @@ module ActionWebPush class SubscriptionsController < ActionController::Base before_action :authenticate_user!, if: :respond_to_authenticate_user? before_action :set_user - before_action :set_subscription, only: [:show, :destroy] + before_action :set_subscription, only: [ :show, :destroy ] def index @subscriptions = ActionWebPush::Subscription.for_user(@user) @@ -18,7 +18,9 @@ def show def create @subscription = ActionWebPush::Subscription.find_or_create_subscription( user: @user, - **subscription_params, + endpoint: subscription_params["endpoint"], + p256dh_key: subscription_params["p256dh_key"], + auth_key: subscription_params["auth_key"], user_agent: request.user_agent ) @@ -57,4 +59,4 @@ def respond_to_authenticate_user? respond_to?(:authenticate_user!) end end -end \ No newline at end of file +end diff --git a/app/models/actionwebpush/subscription.rb b/app/models/action_web_push/subscription.rb similarity index 100% rename from app/models/actionwebpush/subscription.rb rename to app/models/action_web_push/subscription.rb diff --git a/config/routes.rb b/config/routes.rb index 0360ff3..85ee70b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true ActionWebPush::Engine.routes.draw do - resources :subscriptions, only: [:index, :show, :create, :destroy] -end \ No newline at end of file + resources :subscriptions, only: [ :index, :show, :create, :destroy ] +end diff --git a/lib/actionwebpush/delivery_methods/web_push.rb b/lib/actionwebpush/delivery_methods/web_push.rb index edd1c4f..0d143b8 100644 --- a/lib/actionwebpush/delivery_methods/web_push.rb +++ b/lib/actionwebpush/delivery_methods/web_push.rb @@ -21,10 +21,10 @@ def deliver!(notification, connection: nil) urgency: notification.options[:urgency] || "high" ) - payload[:success] = response.success? + payload[:success] = response.kind_of? Net::HTTPSuccess payload[:response_code] = response.code if response.respond_to?(:code) - response.success? + response.kind_of? Net::HTTPSuccess end rescue ::WebPush::ExpiredSubscription => e context = { @@ -71,4 +71,4 @@ def encoded_message(notification) end end end -end \ No newline at end of file +end diff --git a/lib/actionwebpush/instrumentation.rb b/lib/actionwebpush/instrumentation.rb index b3dcfe3..7e54f46 100644 --- a/lib/actionwebpush/instrumentation.rb +++ b/lib/actionwebpush/instrumentation.rb @@ -28,4 +28,4 @@ def self.publish(event_name, payload = {}) # action_web_push.subscription_created # action_web_push.subscription_destroyed end -end \ No newline at end of file +end diff --git a/lib/actionwebpush/notification.rb b/lib/actionwebpush/notification.rb index 740794f..2f9edbe 100644 --- a/lib/actionwebpush/notification.rb +++ b/lib/actionwebpush/notification.rb @@ -26,7 +26,7 @@ def deliver(connection: nil) if subscription authorize_notification_sending!( current_user: @current_user, - subscriptions: [subscription] + subscriptions: [ subscription ] ) end end @@ -94,4 +94,4 @@ def encoded_message JSON.generate(payload) end end -end \ No newline at end of file +end diff --git a/lib/actionwebpush/generators/campfire_migration_generator.rb b/lib/generators/action_web_push/campfire_migration_generator.rb similarity index 99% rename from lib/actionwebpush/generators/campfire_migration_generator.rb rename to lib/generators/action_web_push/campfire_migration_generator.rb index 41e35c6..63416e9 100644 --- a/lib/actionwebpush/generators/campfire_migration_generator.rb +++ b/lib/generators/action_web_push/campfire_migration_generator.rb @@ -66,4 +66,4 @@ def preserve_data? end end end -end \ No newline at end of file +end diff --git a/lib/actionwebpush/generators/install_generator.rb b/lib/generators/action_web_push/install_generator.rb similarity index 76% rename from lib/actionwebpush/generators/install_generator.rb rename to lib/generators/action_web_push/install_generator.rb index 07ec05c..21f5d7c 100644 --- a/lib/actionwebpush/generators/install_generator.rb +++ b/lib/generators/action_web_push/install_generator.rb @@ -3,11 +3,13 @@ require "rails/generators/base" module ActionWebPush - module Generators + module Generators class InstallGenerator < Rails::Generators::Base + include Rails::Generators::Migration + source_root File.expand_path("templates", __dir__) - def create_migration + def create_migration_web_push migration_template( "create_action_web_push_subscriptions.rb", "db/migrate/create_action_web_push_subscriptions.rb", @@ -20,7 +22,7 @@ def create_initializer end def mount_engine - route "mount ActionWebPush::Engine => '/push'" + route "mount ActionWebPush::Engine => '/push'", namespace: [ 'awp' ] end def show_readme @@ -37,11 +39,16 @@ def show_readme private + def self.next_migration_number(dirname) + next_migration_number = current_migration_number(dirname) + 1 + ActiveRecord::Migration.next_migration_number(next_migration_number) + end + def migration_version if Rails::VERSION::MAJOR >= 5 "[#{Rails::VERSION::MAJOR}.#{Rails::VERSION::MINOR}]" end end end - end -end \ No newline at end of file + end +end diff --git a/lib/actionwebpush/generators/templates/campfire_compatibility.rb b/lib/generators/action_web_push/templates/campfire_compatibility.rb similarity index 100% rename from lib/actionwebpush/generators/templates/campfire_compatibility.rb rename to lib/generators/action_web_push/templates/campfire_compatibility.rb diff --git a/lib/actionwebpush/generators/templates/campfire_data_migration.rb b/lib/generators/action_web_push/templates/campfire_data_migration.rb similarity index 100% rename from lib/actionwebpush/generators/templates/campfire_data_migration.rb rename to lib/generators/action_web_push/templates/campfire_data_migration.rb diff --git a/lib/actionwebpush/generators/templates/create_action_web_push_subscriptions.rb b/lib/generators/action_web_push/templates/create_action_web_push_subscriptions.rb similarity index 99% rename from lib/actionwebpush/generators/templates/create_action_web_push_subscriptions.rb rename to lib/generators/action_web_push/templates/create_action_web_push_subscriptions.rb index 6e348b0..1afb623 100644 --- a/lib/actionwebpush/generators/templates/create_action_web_push_subscriptions.rb +++ b/lib/generators/action_web_push/templates/create_action_web_push_subscriptions.rb @@ -14,4 +14,4 @@ def change t.index [:endpoint, :p256dh_key, :auth_key], name: "idx_action_web_push_subscription_keys" end end -end \ No newline at end of file +end diff --git a/lib/actionwebpush/generators/templates/initializer.rb b/lib/generators/action_web_push/templates/initializer.rb similarity index 100% rename from lib/actionwebpush/generators/templates/initializer.rb rename to lib/generators/action_web_push/templates/initializer.rb diff --git a/lib/actionwebpush/generators/vapid_keys_generator.rb b/lib/generators/action_web_push/vapid_keys_generator.rb similarity index 100% rename from lib/actionwebpush/generators/vapid_keys_generator.rb rename to lib/generators/action_web_push/vapid_keys_generator.rb diff --git a/test/comprehensive_test.rb b/test/comprehensive_test.rb index 648be78..cff2bed 100644 --- a/test/comprehensive_test.rb +++ b/test/comprehensive_test.rb @@ -26,10 +26,10 @@ def test_web_push_delivery_with_mock # Mock WebPush.payload_send to avoid actual network call (called twice) mock_response = Minitest::Mock.new - mock_response.expect :success?, true - mock_response.expect :success?, true + mock_response.expect :kind_of?, true, [ Net::HTTPSuccess ] + mock_response.expect :kind_of?, true, [ Net::HTTPSuccess ] - ::WebPush.stub :payload_send, -> (*args) { mock_response } do + ::WebPush.stub :payload_send, ->(*args) { mock_response } do result = web_push_method.deliver!(notification) assert result, "Web push delivery should succeed" end @@ -150,4 +150,4 @@ def test_configuration_defaults assert config.timeout > 0 assert config.max_retries >= 0 end -end \ No newline at end of file +end diff --git a/test/delivery_methods_test.rb b/test/delivery_methods_test.rb index 8ded935..0d2452e 100644 --- a/test/delivery_methods_test.rb +++ b/test/delivery_methods_test.rb @@ -69,10 +69,10 @@ def test_web_push_delivery_method # Mock WebPush response (called twice - for payload and return) mock_response = Minitest::Mock.new - mock_response.expect :success?, true - mock_response.expect :success?, true + mock_response.expect :kind_of?, true, [ Net::HTTPSuccess ] + mock_response.expect :kind_of?, true, [ Net::HTTPSuccess ] - ::WebPush.stub :payload_send, -> (*args) { mock_response } do + ::WebPush.stub :payload_send, ->(*args) { mock_response } do result = web_push_method.deliver!(@notification) assert result end @@ -86,7 +86,7 @@ def test_web_push_delivery_method_with_error web_push_method = ActionWebPush::DeliveryMethods::WebPush.new # Mock WebPush error - ::WebPush.stub :payload_send, -> (*args) { raise ::WebPush::ResponseError.new("Test error") } do + ::WebPush.stub :payload_send, ->(*args) { raise ::WebPush::ResponseError.new("Test error") } do assert_raises(ActionWebPush::DeliveryError) do web_push_method.deliver!(@notification) end @@ -99,7 +99,7 @@ def test_web_push_expired_subscription_handling web_push_method = ActionWebPush::DeliveryMethods::WebPush.new # Mock expired subscription error - simplified for testing - ::WebPush.stub :payload_send, -> (*args) { raise StandardError.new("410 Gone") } do + ::WebPush.stub :payload_send, ->(*args) { raise StandardError.new("410 Gone") } do assert_raises(ActionWebPush::DeliveryError) do web_push_method.deliver!(@notification) end @@ -139,4 +139,4 @@ def deliver!(notification, connection: nil) assert result assert_equal @notification, method.delivered_notification end -end \ No newline at end of file +end