From 29d3960a3ca41bd9e9a665eddb1043b6ead7b003 Mon Sep 17 00:00:00 2001 From: Fernando Olivares Date: Tue, 20 Jan 2026 22:44:06 -0600 Subject: [PATCH] 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 --- ...03313_create_action_push_native_devices.rb | 3 +- db/schema.rb | 3 +- .../controllers/users/devices_controller.rb | 20 +++- saas/lib/fizzy/saas/engine.rb | 4 +- .../users/devices_controller_test.rb | 113 ++++++++++-------- .../fixtures/action_push_native/devices.yml | 3 - 6 files changed, 81 insertions(+), 65 deletions(-) diff --git a/db/migrate/20260114203313_create_action_push_native_devices.rb b/db/migrate/20260114203313_create_action_push_native_devices.rb index e2742dae7..5ef4be322 100644 --- a/db/migrate/20260114203313_create_action_push_native_devices.rb +++ b/db/migrate/20260114203313_create_action_push_native_devices.rb @@ -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 diff --git a/db/schema.rb b/db/schema.rb index c3405d9f9..9ff5edb0c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -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 diff --git a/saas/app/controllers/users/devices_controller.rb b/saas/app/controllers/users/devices_controller.rb index 3b2f8752a..935502408 100644 --- a/saas/app/controllers/users/devices_controller.rb +++ b/saas/app/controllers/users/devices_controller.rb @@ -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 diff --git a/saas/lib/fizzy/saas/engine.rb b/saas/lib/fizzy/saas/engine.rb index fa782414b..3dc1902eb 100644 --- a/saas/lib/fizzy/saas/engine.rb +++ b/saas/lib/fizzy/saas/engine.rb @@ -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 diff --git a/saas/test/controllers/users/devices_controller_test.rb b/saas/test/controllers/users/devices_controller_test.rb index 46da8ce7f..08383c3fe 100644 --- a/saas/test/controllers/users/devices_controller_test.rb +++ b/saas/test/controllers/users/devices_controller_test.rb @@ -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 diff --git a/saas/test/fixtures/action_push_native/devices.yml b/saas/test/fixtures/action_push_native/devices.yml index 0494d2a97..7601d5284 100644 --- a/saas/test/fixtures/action_push_native/devices.yml +++ b/saas/test/fixtures/action_push_native/devices.yml @@ -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