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 <noreply@anthropic.com> Fix reference to `user.devices`, left-over from the identity switch
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
module User::Devices
|
||||
module Identity::Devices
|
||||
extend ActiveSupport::Concern
|
||||
|
||||
included do
|
||||
@@ -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)
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
<div class="native-devices margin-block-start">
|
||||
<h3 class="txt-small txt-uppercase divider">Mobile Devices</h3>
|
||||
|
||||
<% if Current.user.devices.any? %>
|
||||
<% if Current.identity.devices.any? %>
|
||||
<p class="txt-small">
|
||||
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.
|
||||
</p>
|
||||
<%= link_to "Manage devices", devices_path, class: "btn txt-small" %>
|
||||
<% else %>
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user