bterkuile/fizzy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor devices controller and extract registration to model

Browse Source 
- Remove Users namespace from DevicesController (now just DevicesController)
- Create ApplicationPushDevice model extending ActionPushNative::Device
- Move device registration logic (find_or_initialize + update) to model
- Update User::Devices concern to use ApplicationPushDevice
- Fix push notification tests (endpoint validation, job count expectations)
- Update push_config_test to use ActionPushNative.config

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Rosa Gutierrez
2026-01-20 21:40:25 +01:00
parent 7e0470a692
commit 9d32f3e845
9 changed files with 82 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
saas/app/controllers/devices_controller.rb
+28
View File
@@ -0,0 +1,28 @@
class DevicesController < ApplicationController
def index
@devices = Current.user.devices.order(created_at: :desc)
end
def create
ApplicationPushDevice.register(owner: Current.user, **device_params)
head :created
rescue ArgumentError
head :bad_request
end
def destroy
if params[:token].present?
Current.user.devices.destroy_by(token: params[:token])
head :no_content
else
Current.user.devices.destroy_by(id: params[:id])
redirect_to devices_path, notice: "Device removed"
end
end
private
def device_params
params.require([ :token, :platform ])
params.permit(:token, :platform, :name).to_h.symbolize_keys
end
end
saas/app/models/application_push_device.rb
+7
View File
@@ -0,0 +1,7 @@
class ApplicationPushDevice < ActionPushNative::Device
def self.register(owner:, token:, platform:, name: nil)
owner.devices.find_or_initialize_by(token: token).tap do |device|
device.update!(platform: platform.downcase, name: name)
end
end
end
saas/app/models/user/devices.rb
+1 -1
View File
@@ -2,6 +2,6 @@ module User::Devices
extend ActiveSupport::Concern
included do
has_many :devices, class_name: "ActionPushNative::Device", as: :owner, dependent: :destroy
has_many :devices, class_name: "ApplicationPushDevice", as: :owner, dependent: :destroy
end
end
saas/app/views/users/devices/index.html.erb → saas/app/views/devices/index.html.erb
+1 -1
View File
@@ -7,7 +7,7 @@
<strong><%= device.name || "Unnamed device" %></strong>
(<%= device.platform == "apple" ? "iOS" : "Android" %>)
<span>Added <%= time_ago_in_words(device.created_at) %> ago</span>
<%= button_to "Remove", users_device_path(device), method: :delete, data: { confirm: "Remove this device?" } %>
<%= button_to "Remove", device_path(device), method: :delete, data: { confirm: "Remove this device?" } %>
</li>
<% end %>
</ul>
saas/app/views/notifications/settings/_native_devices.html.erb
+1 -1
View File
@@ -5,7 +5,7 @@
<p class="txt-small">
You have <%= pluralize(Current.user.devices.count, "mobile device") %> registered for push notifications.
</p>
<%= link_to "Manage devices", users_devices_path, class: "btn txt-small" %>
<%= link_to "Manage devices", devices_path, class: "btn txt-small" %>
<% else %>
<p class="txt-small color-secondary">
No mobile devices registered. Install the iOS or Android app to receive push notifications on your phone.
saas/lib/fizzy/saas/engine.rb
+2 -4
View File
@@ -44,10 +44,8 @@ module Fizzy
resource :webhooks, only: :create
end
namespace :users do
resources :devices, only: [ :index, :create, :destroy ] do
delete :destroy, on: :collection, as: :unregister
end
resources :devices, only: [ :index, :create, :destroy ] do
delete :destroy, on: :collection, as: :unregister
end
end
end
saas/test/controllers/users/devices_controller_test.rb → saas/test/controllers/devices_controller_test.rb
+21 -21
View File
@@ -1,6 +1,6 @@
require "test_helper"
class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
class DevicesControllerTest < ActionDispatch::IntegrationTest
setup do
@user = users(:david)
sign_in_as @user
@@ -11,7 +11,7 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
test "index shows user devices" do
@user.devices.create!(token: "test_token_123", platform: "apple", name: "iPhone 15 Pro")
get users_devices_path
get devices_path
assert_response :success
assert_select "strong", "iPhone 15 Pro"
@@ -21,7 +21,7 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
test "index shows empty state when no devices" do
@user.devices.delete_all
get users_devices_path
get devices_path
assert_response :success
assert_select "p", /No devices registered/
@@ -30,7 +30,7 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
test "index requires authentication" do
sign_out
get users_devices_path
get devices_path
assert_response :redirect
end
@@ -41,7 +41,7 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
token = SecureRandom.hex(32)
assert_difference "ActionPushNative::Device.count", 1 do
post users_devices_path, params: {
post devices_path, params: {
token: token,
platform: "apple",
name: "iPhone 15 Pro"
@@ -58,7 +58,7 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
end
test "creates android device" do
post users_devices_path, params: {
post devices_path, params: {
token: SecureRandom.hex(32),
platform: "google",
name: "Pixel 8"
@@ -83,7 +83,7 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
# Current user registers the same token with their own device
assert_difference "ActionPushNative::Device.count", 1 do
post users_devices_path, params: {
post devices_path, params: {
token: shared_token,
platform: "apple",
name: "David's iPhone"
@@ -102,7 +102,7 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
end
test "rejects invalid platform" do
post users_devices_path, params: {
post devices_path, params: {
token: SecureRandom.hex(32),
platform: "windows",
name: "Surface"
@@ -112,7 +112,7 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
end
test "rejects missing token" do
post users_devices_path, params: {
post devices_path, params: {
platform: "apple",
name: "iPhone"
}, as: :json
@@ -123,7 +123,7 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
test "create requires authentication" do
sign_out
post users_devices_path, params: {
post devices_path, params: {
token: SecureRandom.hex(32),
platform: "apple"
}, as: :json
@@ -141,19 +141,19 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
)
assert_difference "ActionPushNative::Device.count", -1 do
delete users_device_path(device)
delete device_path(device)
end
assert_redirected_to users_devices_path
assert_redirected_to devices_path
assert_not ActionPushNative::Device.exists?(device.id)
end
test "does nothing when device not found by id" do
assert_no_difference "ActionPushNative::Device.count" do
delete users_device_path(id: "nonexistent")
delete device_path(id: "nonexistent")
end
assert_redirected_to users_devices_path
assert_redirected_to devices_path
end
test "cannot destroy another user's device by id" do
@@ -165,10 +165,10 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
)
assert_no_difference "ActionPushNative::Device.count" do
delete users_device_path(device)
delete device_path(device)
end
assert_redirected_to users_devices_path
assert_redirected_to devices_path
assert ActionPushNative::Device.exists?(device.id)
end
@@ -181,7 +181,7 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
sign_out
delete users_device_path(device)
delete device_path(device)
assert_response :redirect
assert ActionPushNative::Device.exists?(device.id)
@@ -197,7 +197,7 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
)
assert_difference "ActionPushNative::Device.count", -1 do
delete unregister_users_devices_path, params: { token: "token_to_unregister" }, as: :json
delete unregister_devices_path, params: { token: "token_to_unregister" }, as: :json
end
assert_response :no_content
@@ -206,7 +206,7 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
test "does nothing when device not found by token" do
assert_no_difference "ActionPushNative::Device.count" do
delete unregister_users_devices_path, params: { token: "nonexistent_token" }, as: :json
delete unregister_devices_path, params: { token: "nonexistent_token" }, as: :json
end
assert_response :no_content
@@ -221,7 +221,7 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
)
assert_no_difference "ActionPushNative::Device.count" do
delete unregister_users_devices_path, params: { token: "other_users_token" }, as: :json
delete unregister_devices_path, params: { token: "other_users_token" }, as: :json
end
assert_response :no_content
@@ -237,7 +237,7 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
sign_out
delete unregister_users_devices_path, params: { token: "my_token" }, as: :json
delete unregister_devices_path, params: { token: "my_token" }, as: :json
assert_response :redirect
assert ActionPushNative::Device.exists?(device.id)
saas/test/models/notification_pusher_test.rb
+17 -16
View File
@@ -59,14 +59,14 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase
# === Has Any Push Destination ===
test "push_destination returns true when user has native devices" do
@user.devices.create!(uuid: SecureRandom.uuid, token: "test123", platform: "apple", name: "Test iPhone")
@user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone")
assert @pusher.send(:push_destination?)
end
test "push_destination returns true when user has web subscriptions" do
@user.push_subscriptions.create!(
endpoint: "https://example.com/push",
endpoint: "https://fcm.googleapis.com/fcm/send/test",
p256dh_key: "test_p256dh",
auth_key: "test_auth"
)
@@ -85,7 +85,7 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase
test "push delivers to native devices when user has devices" do
stub_push_services
@user.devices.create!(uuid: SecureRandom.uuid, token: "test123", platform: "apple", name: "Test iPhone")
@user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone")
assert_native_push_delivery(count: 1) do
@pusher.push
@@ -102,7 +102,7 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase
test "push does not deliver when creator is system user" do
stub_push_services
@user.devices.create!(uuid: SecureRandom.uuid, token: "test123", platform: "apple", name: "Test iPhone")
@user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone")
@notification.update!(creator: users(:system))
result = @pusher.push
@@ -112,10 +112,11 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase
test "push delivers to multiple devices" do
stub_push_services
@user.devices.create!(uuid: SecureRandom.uuid, token: "token1", platform: "apple", name: "iPhone")
@user.devices.create!(uuid: SecureRandom.uuid, token: "token2", platform: "google", name: "Pixel")
@user.devices.delete_all
@user.devices.create!(token: "token1", platform: "apple", name: "iPhone")
@user.devices.create!(token: "token2", platform: "google", name: "Pixel")
assert_native_push_delivery(count: 1) do
assert_native_push_delivery(count: 2) do
@pusher.push
end
end
@@ -131,7 +132,7 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase
)
# Set up native device
@user.devices.create!(uuid: SecureRandom.uuid, token: "native_token", platform: "apple", name: "iPhone")
@user.devices.create!(token: "native_token", platform: "apple", name: "iPhone")
# Mock web push pool to verify it receives the payload
web_push_pool = mock("web_push_pool")
@@ -149,7 +150,7 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase
# === Native Notification Building ===
test "native notification includes required fields" do
@user.devices.create!(uuid: SecureRandom.uuid, token: "test123", platform: "apple", name: "Test iPhone")
@user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone")
payload = @pusher.send(:build_payload)
native = @pusher.send(:native_notification, payload)
@@ -159,7 +160,7 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase
end
test "native notification sets thread_id from card" do
@user.devices.create!(uuid: SecureRandom.uuid, token: "test123", platform: "apple", name: "Test iPhone")
@user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone")
payload = @pusher.send(:build_payload)
native = @pusher.send(:native_notification, payload)
@@ -168,7 +169,7 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase
test "native notification sets high_priority for assignments" do
notification = notifications(:logo_assignment_kevin)
notification.user.devices.create!(uuid: SecureRandom.uuid, token: "test123", platform: "apple", name: "Test iPhone")
notification.user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone")
pusher = NotificationPusher.new(notification)
payload = pusher.send(:build_payload)
@@ -178,7 +179,7 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase
end
test "native notification sets normal priority for non-assignments" do
@user.devices.create!(uuid: SecureRandom.uuid, token: "test123", platform: "apple", name: "Test iPhone")
@user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone")
payload = @pusher.send(:build_payload)
native = @pusher.send(:native_notification, payload)
@@ -188,7 +189,7 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase
# === Apple-specific Payload ===
test "native notification includes apple-specific fields" do
@user.devices.create!(uuid: SecureRandom.uuid, token: "test123", platform: "apple", name: "Test iPhone")
@user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone")
payload = @pusher.send(:build_payload)
native = @pusher.send(:native_notification, payload)
@@ -200,7 +201,7 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase
# === Google-specific Payload ===
test "native notification sets android notification to nil for data-only" do
@user.devices.create!(uuid: SecureRandom.uuid, token: "test123", platform: "apple", name: "Test iPhone")
@user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone")
payload = @pusher.send(:build_payload)
native = @pusher.send(:native_notification, payload)
@@ -210,11 +211,11 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase
# === Data Payload ===
test "native notification includes data payload" do
@user.devices.create!(uuid: SecureRandom.uuid, token: "test123", platform: "apple", name: "Test iPhone")
@user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone")
payload = @pusher.send(:build_payload)
native = @pusher.send(:native_notification, payload)
assert_not_nil native.data[:path]
assert_not_nil native.data[:url]
assert_equal @notification.account.external_account_id, native.data[:account_id]
assert_equal @notification.creator.name, native.data[:creator_name]
end
saas/test/models/push_config_test.rb
+4 -4
View File
@@ -4,11 +4,11 @@ class PushConfigTest < ActiveSupport::TestCase
test "loads push config from the saas engine" do
skip unless Fizzy.saas?
config = Rails.application.config_for(:push)
config = ActionPushNative.config
apple_team_id = config.dig("apple", "team_id")
apple_topic = config.dig("apple", "topic")
google_project_id = config.dig("google", "project_id")
apple_team_id = config.dig(:apple, :team_id)
apple_topic = config.dig(:apple, :topic)
google_project_id = config.dig(:google, :project_id)
skip "Update test once APNS team_id is configured" if apple_team_id == "YOUR_TEAM_ID"
skip "Update test once APNS topic is configured" if apple_topic == "com.yourcompany.fizzy"