Remove UUID requirement from push notification device registration
- Remove UUID column from devices table entirely - Update unique index from (owner, uuid) to (owner, token) - Simplify create action to just create device records - Add token-based unregister route for API clients - Consolidate error handling with rescue_from - Update fixtures to remove uuid references Devices are now identified by (owner, token) instead of UUID. This simplifies the client-side registration flow. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
committed by
Rosa Gutierrez
parent
2121b6dc3d
commit
29d3960a3c
@@ -1,7 +1,6 @@
|
||||
class CreateActionPushNativeDevices < ActiveRecord::Migration[8.0]
|
||||
def change
|
||||
create_table :action_push_native_devices do |t|
|
||||
t.string :uuid, null: false
|
||||
t.string :name
|
||||
t.string :platform, null: false
|
||||
t.string :token, null: false
|
||||
@@ -10,6 +9,6 @@ class CreateActionPushNativeDevices < ActiveRecord::Migration[8.0]
|
||||
t.timestamps
|
||||
end
|
||||
|
||||
add_index :action_push_native_devices, [ :owner_type, :owner_id, :uuid ], unique: true
|
||||
add_index :action_push_native_devices, [ :owner_type, :owner_id, :token ], unique: true
|
||||
end
|
||||
end
|
||||
|
||||
Generated
+1
-2
@@ -77,8 +77,7 @@ ActiveRecord::Schema[8.2].define(version: 2026_02_18_120000) do
|
||||
t.string "platform", null: false
|
||||
t.string "token", null: false
|
||||
t.datetime "updated_at", null: false
|
||||
t.string "uuid", null: false
|
||||
t.index ["owner_type", "owner_id", "uuid"], name: "idx_on_owner_type_owner_id_uuid_a42e3920d5", unique: true
|
||||
t.index ["owner_type", "owner_id", "token"], name: "idx_on_owner_type_owner_id_token_95a4008c64", unique: true
|
||||
t.index ["owner_type", "owner_id"], name: "index_action_push_native_devices_on_owner"
|
||||
end
|
||||
|
||||
|
||||
@@ -1,20 +1,24 @@
|
||||
class Users::DevicesController < ApplicationController
|
||||
before_action :set_devices
|
||||
|
||||
rescue_from ActiveRecord::NotNullViolation, ArgumentError, with: :bad_request
|
||||
|
||||
def index
|
||||
end
|
||||
|
||||
def create
|
||||
device = @devices.find_or_initialize_by(uuid: params.require(:uuid))
|
||||
device.update!(device_params)
|
||||
@devices.create!(device_params)
|
||||
head :created
|
||||
rescue ArgumentError
|
||||
head :bad_request
|
||||
end
|
||||
|
||||
def destroy
|
||||
@devices.destroy_by(id: params[:id])
|
||||
redirect_to users_devices_path, notice: "Device removed"
|
||||
if params[:token].present?
|
||||
@devices.destroy_by(token: params[:token])
|
||||
head :no_content
|
||||
else
|
||||
@devices.destroy_by(id: params[:id])
|
||||
redirect_to users_devices_path, notice: "Device removed"
|
||||
end
|
||||
end
|
||||
|
||||
private
|
||||
@@ -27,4 +31,8 @@ class Users::DevicesController < ApplicationController
|
||||
permitted[:platform] = permitted[:platform].to_s.downcase if permitted[:platform].present?
|
||||
end
|
||||
end
|
||||
|
||||
def bad_request
|
||||
head :bad_request
|
||||
end
|
||||
end
|
||||
|
||||
@@ -45,7 +45,9 @@ module Fizzy
|
||||
end
|
||||
|
||||
namespace :users do
|
||||
resources :devices, only: [ :index, :create, :destroy ]
|
||||
resources :devices, only: [ :index, :create, :destroy ] do
|
||||
delete :destroy, on: :collection, as: :unregister
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -9,7 +9,7 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
|
||||
# === Index (Web) ===
|
||||
|
||||
test "index shows user devices" do
|
||||
@user.devices.create!(uuid: "test-uuid", token: "test_token_123", platform: "apple", name: "iPhone 15 Pro")
|
||||
@user.devices.create!(token: "test_token_123", platform: "apple", name: "iPhone 15 Pro")
|
||||
|
||||
get users_devices_path
|
||||
|
||||
@@ -38,12 +38,10 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
|
||||
# === Create (API) ===
|
||||
|
||||
test "creates a new device via api" do
|
||||
uuid = SecureRandom.uuid
|
||||
token = SecureRandom.hex(32)
|
||||
|
||||
assert_difference "ActionPushNative::Device.count", 1 do
|
||||
post users_devices_path, params: {
|
||||
uuid: uuid,
|
||||
token: token,
|
||||
platform: "apple",
|
||||
name: "iPhone 15 Pro"
|
||||
@@ -53,7 +51,6 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
|
||||
assert_response :created
|
||||
|
||||
device = ActionPushNative::Device.last
|
||||
assert_equal uuid, device.uuid
|
||||
assert_equal token, device.token
|
||||
assert_equal "apple", device.platform
|
||||
assert_equal "iPhone 15 Pro", device.name
|
||||
@@ -62,7 +59,6 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
|
||||
|
||||
test "creates android device" do
|
||||
post users_devices_path, params: {
|
||||
uuid: SecureRandom.uuid,
|
||||
token: SecureRandom.hex(32),
|
||||
platform: "google",
|
||||
name: "Pixel 8"
|
||||
@@ -74,36 +70,12 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
|
||||
assert_equal "google", device.platform
|
||||
end
|
||||
|
||||
test "updates existing device with same uuid" do
|
||||
existing_device = @user.devices.create!(
|
||||
uuid: "my-device-uuid",
|
||||
token: "old_token",
|
||||
platform: "apple",
|
||||
name: "Old iPhone"
|
||||
)
|
||||
|
||||
assert_no_difference "ActionPushNative::Device.count" do
|
||||
post users_devices_path, params: {
|
||||
uuid: "my-device-uuid",
|
||||
token: "new_token",
|
||||
platform: "apple",
|
||||
name: "New iPhone"
|
||||
}, as: :json
|
||||
end
|
||||
|
||||
assert_response :created
|
||||
existing_device.reload
|
||||
assert_equal "new_token", existing_device.token
|
||||
assert_equal "New iPhone", existing_device.name
|
||||
end
|
||||
|
||||
test "same token can be registered by multiple users" do
|
||||
shared_token = "shared_push_token_123"
|
||||
other_user = users(:kevin)
|
||||
|
||||
# Other user registers the token first
|
||||
other_device = other_user.devices.create!(
|
||||
uuid: "kevins-device-uuid",
|
||||
token: shared_token,
|
||||
platform: "apple",
|
||||
name: "Kevin's iPhone"
|
||||
@@ -112,7 +84,6 @@ 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: {
|
||||
uuid: "davids-device-uuid",
|
||||
token: shared_token,
|
||||
platform: "apple",
|
||||
name: "David's iPhone"
|
||||
@@ -125,14 +96,13 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
|
||||
assert_equal shared_token, other_device.reload.token
|
||||
assert_equal other_user, other_device.owner
|
||||
|
||||
davids_device = @user.devices.find_by(uuid: "davids-device-uuid")
|
||||
davids_device = @user.devices.last
|
||||
assert_equal shared_token, davids_device.token
|
||||
assert_equal @user, davids_device.owner
|
||||
end
|
||||
|
||||
test "rejects invalid platform" do
|
||||
post users_devices_path, params: {
|
||||
uuid: SecureRandom.uuid,
|
||||
token: SecureRandom.hex(32),
|
||||
platform: "windows",
|
||||
name: "Surface"
|
||||
@@ -141,19 +111,8 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
|
||||
assert_response :bad_request
|
||||
end
|
||||
|
||||
test "rejects missing uuid" do
|
||||
post users_devices_path, params: {
|
||||
token: SecureRandom.hex(32),
|
||||
platform: "apple",
|
||||
name: "iPhone"
|
||||
}, as: :json
|
||||
|
||||
assert_response :bad_request
|
||||
end
|
||||
|
||||
test "rejects missing token" do
|
||||
post users_devices_path, params: {
|
||||
uuid: SecureRandom.uuid,
|
||||
platform: "apple",
|
||||
name: "iPhone"
|
||||
}, as: :json
|
||||
@@ -165,7 +124,6 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
|
||||
sign_out
|
||||
|
||||
post users_devices_path, params: {
|
||||
uuid: SecureRandom.uuid,
|
||||
token: SecureRandom.hex(32),
|
||||
platform: "apple"
|
||||
}, as: :json
|
||||
@@ -175,9 +133,8 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
|
||||
|
||||
# === Destroy (Web) ===
|
||||
|
||||
test "destroys device" do
|
||||
test "destroys device by id" do
|
||||
device = @user.devices.create!(
|
||||
uuid: "device-to-delete",
|
||||
token: "token_to_delete",
|
||||
platform: "apple",
|
||||
name: "iPhone"
|
||||
@@ -191,7 +148,7 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
|
||||
assert_not ActionPushNative::Device.exists?(device.id)
|
||||
end
|
||||
|
||||
test "does nothing when device not found" do
|
||||
test "does nothing when device not found by id" do
|
||||
assert_no_difference "ActionPushNative::Device.count" do
|
||||
delete users_device_path(id: "nonexistent")
|
||||
end
|
||||
@@ -199,10 +156,9 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
|
||||
assert_redirected_to users_devices_path
|
||||
end
|
||||
|
||||
test "cannot destroy another user's device" do
|
||||
test "cannot destroy another user's device by id" do
|
||||
other_user = users(:kevin)
|
||||
device = other_user.devices.create!(
|
||||
uuid: "other-users-device",
|
||||
token: "other_users_token",
|
||||
platform: "apple",
|
||||
name: "Other iPhone"
|
||||
@@ -216,9 +172,8 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
|
||||
assert ActionPushNative::Device.exists?(device.id)
|
||||
end
|
||||
|
||||
test "destroy requires authentication" do
|
||||
test "destroy by id requires authentication" do
|
||||
device = @user.devices.create!(
|
||||
uuid: "my-device",
|
||||
token: "my_token",
|
||||
platform: "apple",
|
||||
name: "iPhone"
|
||||
@@ -231,4 +186,60 @@ class Users::DevicesControllerTest < ActionDispatch::IntegrationTest
|
||||
assert_response :redirect
|
||||
assert ActionPushNative::Device.exists?(device.id)
|
||||
end
|
||||
|
||||
# === Destroy by Token (API) ===
|
||||
|
||||
test "destroys device by token" do
|
||||
device = @user.devices.create!(
|
||||
token: "token_to_unregister",
|
||||
platform: "apple",
|
||||
name: "iPhone"
|
||||
)
|
||||
|
||||
assert_difference "ActionPushNative::Device.count", -1 do
|
||||
delete unregister_users_devices_path, params: { token: "token_to_unregister" }, as: :json
|
||||
end
|
||||
|
||||
assert_response :no_content
|
||||
assert_not ActionPushNative::Device.exists?(device.id)
|
||||
end
|
||||
|
||||
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
|
||||
end
|
||||
|
||||
assert_response :no_content
|
||||
end
|
||||
|
||||
test "cannot destroy another user's device by token" do
|
||||
other_user = users(:kevin)
|
||||
device = other_user.devices.create!(
|
||||
token: "other_users_token",
|
||||
platform: "apple",
|
||||
name: "Other iPhone"
|
||||
)
|
||||
|
||||
assert_no_difference "ActionPushNative::Device.count" do
|
||||
delete unregister_users_devices_path, params: { token: "other_users_token" }, as: :json
|
||||
end
|
||||
|
||||
assert_response :no_content
|
||||
assert ActionPushNative::Device.exists?(device.id)
|
||||
end
|
||||
|
||||
test "destroy by token requires authentication" do
|
||||
device = @user.devices.create!(
|
||||
token: "my_token",
|
||||
platform: "apple",
|
||||
name: "iPhone"
|
||||
)
|
||||
|
||||
sign_out
|
||||
|
||||
delete unregister_users_devices_path, params: { token: "my_token" }, as: :json
|
||||
|
||||
assert_response :redirect
|
||||
assert ActionPushNative::Device.exists?(device.id)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -1,19 +1,16 @@
|
||||
davids_iphone:
|
||||
uuid: device-uuid-davids-iphone
|
||||
name: iPhone 15 Pro
|
||||
token: abc123def456abc123def456abc123def456abc123def456abc123def456abcd
|
||||
platform: apple
|
||||
owner: david (User)
|
||||
|
||||
davids_pixel:
|
||||
uuid: device-uuid-davids-pixel
|
||||
name: Pixel 8
|
||||
token: def456abc123def456abc123def456abc123def456abc123def456abc123defg
|
||||
platform: google
|
||||
owner: david (User)
|
||||
|
||||
kevins_iphone:
|
||||
uuid: device-uuid-kevins-iphone
|
||||
name: iPhone 14
|
||||
token: 789xyz789xyz789xyz789xyz789xyz789xyz789xyz789xyz789xyz789xyz7890
|
||||
platform: apple
|
||||
|
||||
Reference in New Issue
Block a user