From 555132bbef8eef9896c2dfce22c864519ad6aa1d Mon Sep 17 00:00:00 2001 From: Rosa Gutierrez Date: Wed, 21 Jan 2026 15:33:17 +0100 Subject: [PATCH] Change device ownership from User to Identity Devices now belong to Identity instead of User, allowing a single device registration to work across all accounts an identity has access to. - Move User::Devices to Identity::Devices - Update DevicesController to use Current.identity - Update NotificationPusher::Native to use user.identity.devices - Clean up tests to use @identity directly Co-Authored-By: Claude Opus 4.5 Fix reference to `user.devices`, left-over from the identity switch --- saas/app/controllers/devices_controller.rb | 6 +- saas/app/models/{user => identity}/devices.rb | 2 +- saas/app/models/notification_pusher/native.rb | 4 +- .../settings/_native_devices.html.erb | 4 +- saas/lib/fizzy/saas/engine.rb | 2 +- .../controllers/devices_controller_test.rb | 88 +++++++++---------- saas/test/models/notification_pusher_test.rb | 51 ++++------- 7 files changed, 71 insertions(+), 86 deletions(-) rename saas/app/models/{user => identity}/devices.rb (85%) diff --git a/saas/app/controllers/devices_controller.rb b/saas/app/controllers/devices_controller.rb index 081c322b8..6913da0c5 100644 --- a/saas/app/controllers/devices_controller.rb +++ b/saas/app/controllers/devices_controller.rb @@ -2,11 +2,11 @@ class DevicesController < ApplicationController before_action :set_device, only: :destroy def index - @devices = Current.user.devices.order(created_at: :desc) + @devices = Current.identity.devices.order(created_at: :desc) end def create - ApplicationPushDevice.register(owner: Current.user, **device_params) + ApplicationPushDevice.register(owner: Current.identity, **device_params) head :created end @@ -20,7 +20,7 @@ class DevicesController < ApplicationController private def set_device - @device = Current.user.devices.find_by(token: params[:id]) || Current.user.devices.find(params[:id]) + @device = Current.identity.devices.find_by(token: params[:id]) || Current.identity.devices.find(params[:id]) end def device_params diff --git a/saas/app/models/user/devices.rb b/saas/app/models/identity/devices.rb similarity index 85% rename from saas/app/models/user/devices.rb rename to saas/app/models/identity/devices.rb index df198df16..ce7eec457 100644 --- a/saas/app/models/user/devices.rb +++ b/saas/app/models/identity/devices.rb @@ -1,4 +1,4 @@ -module User::Devices +module Identity::Devices extend ActiveSupport::Concern included do diff --git a/saas/app/models/notification_pusher/native.rb b/saas/app/models/notification_pusher/native.rb index e5f83e40a..2fc38ed72 100644 --- a/saas/app/models/notification_pusher/native.rb +++ b/saas/app/models/notification_pusher/native.rb @@ -12,11 +12,11 @@ module NotificationPusher::Native private def push_destination? - notification.user.push_subscriptions.any? || notification.user.devices.any? + notification.user.push_subscriptions.any? || notification.user.identity.devices.any? end def push_to_native(payload) - devices = notification.user.devices + devices = notification.user.identity.devices return if devices.empty? native_notification(payload).deliver_later_to(devices) diff --git a/saas/app/views/notifications/settings/_native_devices.html.erb b/saas/app/views/notifications/settings/_native_devices.html.erb index 9931f822f..ea9e9987d 100644 --- a/saas/app/views/notifications/settings/_native_devices.html.erb +++ b/saas/app/views/notifications/settings/_native_devices.html.erb @@ -1,9 +1,9 @@

Mobile Devices

- <% if Current.user.devices.any? %> + <% if Current.identity.devices.any? %>

- You have <%= pluralize(Current.user.devices.count, "mobile device") %> registered for push notifications. + You have <%= pluralize(Current.identity.devices.count, "mobile device") %> registered for push notifications.

<%= link_to "Manage devices", devices_path, class: "btn txt-small" %> <% else %> diff --git a/saas/lib/fizzy/saas/engine.rb b/saas/lib/fizzy/saas/engine.rb index e2e86c033..31fadee33 100644 --- a/saas/lib/fizzy/saas/engine.rb +++ b/saas/lib/fizzy/saas/engine.rb @@ -154,7 +154,7 @@ module Fizzy config.to_prepare do ::Account.include Account::Billing, Account::Limited ::User.include User::NotifiesAccountOfEmailChange - ::User.include User::Devices + ::Identity.include Identity::Devices ::NotificationPusher.prepend NotificationPusher::Native ::Signup.prepend Fizzy::Saas::Signup CardsController.include(Card::LimitedCreation) diff --git a/saas/test/controllers/devices_controller_test.rb b/saas/test/controllers/devices_controller_test.rb index 0cfbbd612..0434a29cd 100644 --- a/saas/test/controllers/devices_controller_test.rb +++ b/saas/test/controllers/devices_controller_test.rb @@ -2,12 +2,12 @@ require "test_helper" class DevicesControllerTest < ActionDispatch::IntegrationTest setup do - @user = users(:david) - sign_in_as @user + @identity = identities(:david) + sign_in_as :david end - test "index shows user devices" do - @user.devices.create!(token: "test_token_123", platform: "apple", name: "iPhone 15 Pro") + test "index shows identity's devices" do + @identity.devices.create!(token: "test_token_123", platform: "apple", name: "iPhone 15 Pro") get devices_path @@ -17,7 +17,7 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest end test "index shows empty state when no devices" do - @user.devices.delete_all + @identity.devices.delete_all get devices_path @@ -36,7 +36,7 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest test "creates a new device via api" do token = SecureRandom.hex(32) - assert_difference "ActionPushNative::Device.count", 1 do + assert_difference -> { ApplicationPushDevice.count }, 1 do post devices_path, params: { token: token, platform: "apple", @@ -46,11 +46,11 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest assert_response :created - device = ActionPushNative::Device.last + device = ApplicationPushDevice.last assert_equal token, device.token assert_equal "apple", device.platform assert_equal "iPhone 15 Pro", device.name - assert_equal @user, device.owner + assert_equal @identity, device.owner end test "creates android device" do @@ -62,23 +62,23 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest assert_response :created - device = ActionPushNative::Device.last + device = ApplicationPushDevice.last assert_equal "google", device.platform end - test "same token can be registered by multiple users" do + test "same token can be registered by multiple identities" do shared_token = "shared_push_token_123" - other_user = users(:kevin) + other_identity = identities(:kevin) - # Other user registers the token first - other_device = other_user.devices.create!( + # Other identity registers the token first + other_device = other_identity.devices.create!( token: shared_token, platform: "apple", name: "Kevin's iPhone" ) - # Current user registers the same token with their own device - assert_difference "ActionPushNative::Device.count", 1 do + # Current identity registers the same token with their own device + assert_difference -> { ApplicationPushDevice.count }, 1 do post devices_path, params: { token: shared_token, platform: "apple", @@ -88,13 +88,13 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest assert_response :created - # Both users have their own device records + # Both identities have their own device records assert_equal shared_token, other_device.reload.token - assert_equal other_user, other_device.owner + assert_equal other_identity, other_device.owner - davids_device = @user.devices.last + davids_device = @identity.devices.last assert_equal shared_token, davids_device.token - assert_equal @user, davids_device.owner + assert_equal @identity, davids_device.owner end test "rejects invalid platform" do @@ -128,46 +128,46 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest end test "destroys device by id" do - device = @user.devices.create!( + device = @identity.devices.create!( token: "token_to_delete", platform: "apple", name: "iPhone" ) - assert_difference "ActionPushNative::Device.count", -1 do + assert_difference -> { ApplicationPushDevice.count }, -1 do delete device_path(device) end assert_redirected_to devices_path - assert_not ActionPushNative::Device.exists?(device.id) + assert_not ApplicationPushDevice.exists?(device.id) end test "returns not found when device not found by id" do - assert_no_difference "ActionPushNative::Device.count" do + assert_no_difference "ApplicationPushDevice.count" do delete device_path(id: "nonexistent") end assert_response :not_found end - test "returns not found for another user's device by id" do - other_user = users(:kevin) - device = other_user.devices.create!( - token: "other_users_token", + test "returns not found for another identity's device by id" do + other_identity = identities(:kevin) + device = other_identity.devices.create!( + token: "other_identity_token", platform: "apple", name: "Other iPhone" ) - assert_no_difference "ActionPushNative::Device.count" do + assert_no_difference "ApplicationPushDevice.count" do delete device_path(device) end assert_response :not_found - assert ActionPushNative::Device.exists?(device.id) + assert ApplicationPushDevice.exists?(device.id) end test "destroy by id requires authentication" do - device = @user.devices.create!( + device = @identity.devices.create!( token: "my_token", platform: "apple", name: "iPhone" @@ -178,50 +178,50 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest delete device_path(device) assert_response :redirect - assert ActionPushNative::Device.exists?(device.id) + assert ApplicationPushDevice.exists?(device.id) end test "destroys device by token" do - device = @user.devices.create!( + device = @identity.devices.create!( token: "token_to_unregister", platform: "apple", name: "iPhone" ) - assert_difference "ActionPushNative::Device.count", -1 do + assert_difference -> { ApplicationPushDevice.count }, -1 do delete device_path("token_to_unregister"), as: :json end assert_response :no_content - assert_not ActionPushNative::Device.exists?(device.id) + assert_not ApplicationPushDevice.exists?(device.id) end test "returns not found when device not found by token" do - assert_no_difference "ActionPushNative::Device.count" do + assert_no_difference "ApplicationPushDevice.count" do delete device_path("nonexistent_token"), as: :json end assert_response :not_found end - test "returns not found for another user's device by token" do - other_user = users(:kevin) - device = other_user.devices.create!( - token: "other_users_token", + test "returns not found for another identity's device by token" do + other_identity = identities(:kevin) + device = other_identity.devices.create!( + token: "other_identity_token", platform: "apple", name: "Other iPhone" ) - assert_no_difference "ActionPushNative::Device.count" do - delete device_path("other_users_token"), as: :json + assert_no_difference "ApplicationPushDevice.count" do + delete device_path("other_identity_token"), as: :json end assert_response :not_found - assert ActionPushNative::Device.exists?(device.id) + assert ApplicationPushDevice.exists?(device.id) end test "destroy by token requires authentication" do - device = @user.devices.create!( + device = @identity.devices.create!( token: "my_token", platform: "apple", name: "iPhone" @@ -232,6 +232,6 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest delete device_path("my_token"), as: :json assert_response :redirect - assert ActionPushNative::Device.exists?(device.id) + assert ApplicationPushDevice.exists?(device.id) end end diff --git a/saas/test/models/notification_pusher_test.rb b/saas/test/models/notification_pusher_test.rb index 70a9f90ef..ef7fcd0d5 100644 --- a/saas/test/models/notification_pusher_test.rb +++ b/saas/test/models/notification_pusher_test.rb @@ -3,6 +3,7 @@ require "test_helper" class NotificationPusherNativeTest < ActiveSupport::TestCase setup do @user = users(:kevin) + @identity = @user.identity @notification = notifications(:logo_published_kevin) @pusher = NotificationPusher.new(@notification) @@ -10,8 +11,6 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase @user.push_subscriptions.delete_all end - # === Notification Category === - test "notification_category returns assignment for card_assigned" do notification = notifications(:logo_assignment_kevin) pusher = NotificationPusher.new(notification) @@ -40,8 +39,6 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase assert_equal "card", pusher.send(:notification_category) end - # === Interruption Level === - test "interruption_level is time-sensitive for assignments" do notification = notifications(:logo_assignment_kevin) pusher = NotificationPusher.new(notification) @@ -56,10 +53,8 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase assert_equal "active", pusher.send(:interruption_level) end - # === Has Any Push Destination === - - test "push_destination returns true when user has native devices" do - @user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") + test "push_destination returns true when identity has native devices" do + @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") assert @pusher.send(:push_destination?) end @@ -75,17 +70,15 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase end test "push_destination returns false when user has neither" do - @user.devices.delete_all + @identity.devices.delete_all @user.push_subscriptions.delete_all assert_not @pusher.send(:push_destination?) end - # === Push Delivery === - test "push delivers to native devices when user has devices" do stub_push_services - @user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") + @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") assert_native_push_delivery(count: 1) do @pusher.push @@ -93,7 +86,7 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase end test "push does not deliver to native when user has no devices" do - @user.devices.delete_all + @identity.devices.delete_all assert_no_native_push_delivery do @pusher.push @@ -102,7 +95,7 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase test "push does not deliver when creator is system user" do stub_push_services - @user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") + @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") @notification.update!(creator: users(:system)) result = @pusher.push @@ -112,9 +105,9 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase test "push delivers to multiple devices" do stub_push_services - @user.devices.delete_all - @user.devices.create!(token: "token1", platform: "apple", name: "iPhone") - @user.devices.create!(token: "token2", platform: "google", name: "Pixel") + @identity.devices.delete_all + @identity.devices.create!(token: "token1", platform: "apple", name: "iPhone") + @identity.devices.create!(token: "token2", platform: "google", name: "Pixel") assert_native_push_delivery(count: 2) do @pusher.push @@ -132,7 +125,7 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase ) # Set up native device - @user.devices.create!(token: "native_token", platform: "apple", name: "iPhone") + @identity.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") @@ -147,10 +140,8 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase end end - # === Native Notification Building === - test "native notification includes required fields" do - @user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") + @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") payload = @pusher.send(:build_payload) native = @pusher.send(:native_notification, payload) @@ -160,7 +151,7 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase end test "native notification sets thread_id from card" do - @user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") + @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") payload = @pusher.send(:build_payload) native = @pusher.send(:native_notification, payload) @@ -169,7 +160,7 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase test "native notification sets high_priority for assignments" do notification = notifications(:logo_assignment_kevin) - notification.user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") + notification.user.identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") pusher = NotificationPusher.new(notification) payload = pusher.send(:build_payload) @@ -179,17 +170,15 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase end test "native notification sets normal priority for non-assignments" do - @user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") + @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") payload = @pusher.send(:build_payload) native = @pusher.send(:native_notification, payload) assert_not native.high_priority end - # === Apple-specific Payload === - test "native notification includes apple-specific fields" do - @user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") + @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") payload = @pusher.send(:build_payload) native = @pusher.send(:native_notification, payload) @@ -198,20 +187,16 @@ class NotificationPusherNativeTest < ActiveSupport::TestCase assert_not_nil native.apple_data.dig(:aps, :category) end - # === Google-specific Payload === - test "native notification sets android notification to nil for data-only" do - @user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") + @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") payload = @pusher.send(:build_payload) native = @pusher.send(:native_notification, payload) assert_nil native.google_data.dig(:android, :notification) end - # === Data Payload === - test "native notification includes data payload" do - @user.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") + @identity.devices.create!(token: "test123", platform: "apple", name: "Test iPhone") payload = @pusher.send(:build_payload) native = @pusher.send(:native_notification, payload)