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

Simplify device routes and use ActiveRecord validations

Browse Source 
- Use RESTful DELETE /devices/:id where :id can be token or database ID
- Remove redundant unregister collection route
- Remove old Users::DevicesController
- Return 404 when device not found instead of silently succeeding
- Return 422 for invalid platform via ActiveRecord validation
- Update action_push_native to main branch (includes validate: true on enum)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Rosa Gutierrez
2026-01-21 13:20:14 +01:00
parent 9d32f3e845
commit d3cc79a5bc
6 changed files with 30 additions and 77 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Gemfile.saas
+1 -1
View File
@@ -12,7 +12,7 @@ gem "console1984", bc: "console1984"
gem "audits1984", bc: "audits1984", branch: "flavorjones/coworker-api"
# Native push notifications (iOS/Android)
gem "action_push_native", github: "rails/action_push_native", branch: "use-registered-config-path"
gem "action_push_native", github: "rails/action_push_native"
# Telemetry
gem "rails_structured_logging", bc: "rails-structured-logging"
Gemfile.saas.lock
+5 -6
View File
@@ -62,8 +62,7 @@ GIT
GIT
remote: https://github.com/rails/action_push_native.git
revision: d5c44514e13faf919261ca7943fc43aedd8e992e
branch: use-registered-config-path
revision: 9fb4a2bfe54270b1a3508028f00aaa586e257655
specs:
action_push_native (0.3.0)
activejob (>= 8.0)
@@ -221,8 +220,8 @@ GEM
activemodel (>= 7.0)
activemodel-serializers-xml (~> 1.0)
activesupport (>= 7.0)
addressable (2.8.7)
public_suffix (>= 2.0.2, < 7.0)
addressable (2.8.8)
public_suffix (>= 2.0.2, < 8.0)
anyway_config (2.7.2)
ruby-next-core (~> 1.0)
ast (2.4.3)
@@ -319,7 +318,7 @@ GEM
base64 (~> 0.2)
faraday (>= 1.0, < 3.a)
google-logging-utils (0.2.0)
googleauth (1.16.0)
googleauth (1.16.1)
faraday (>= 1.0, < 3.a)
google-cloud-env (~> 2.2)
google-logging-utils (~> 0.1)
@@ -495,7 +494,7 @@ GEM
psych (5.3.1)
date
stringio
public_suffix (6.0.2)
public_suffix (7.0.2)
puma (7.2.0)
nio4r (~> 2.0)
raabro (1.4.0)
saas/app/controllers/devices_controller.rb
+10 -8
View File
@@ -1,4 +1,6 @@
class DevicesController < ApplicationController
before_action :set_device, only: :destroy
def index
@devices = Current.user.devices.order(created_at: :desc)
end
@@ -6,21 +8,21 @@ class DevicesController < ApplicationController
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"
@device.destroy
respond_to do |format|
format.html { redirect_to devices_path, notice: "Device removed" }
format.json { head :no_content }
end
end
private
def set_device
@device = Current.user.devices.find_by(token: params[:id]) || Current.user.devices.find(params[:id])
end
def device_params
params.require([ :token, :platform ])
params.permit(:token, :platform, :name).to_h.symbolize_keys
saas/app/controllers/users/devices_controller.rb
-38
View File
@@ -1,38 +0,0 @@
class Users::DevicesController < ApplicationController
before_action :set_devices
rescue_from ActiveRecord::NotNullViolation, ArgumentError, with: :bad_request
def index
end
def create
@devices.create!(device_params)
head :created
end
def destroy
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
def set_devices
@devices = Current.user.devices.order(created_at: :desc)
end
def device_params
params.permit(:token, :platform, :name).tap do |permitted|
permitted[:platform] = permitted[:platform].to_s.downcase if permitted[:platform].present?
end
end
def bad_request
head :bad_request
end
end
saas/lib/fizzy/saas/engine.rb
+1 -3
View File
@@ -44,9 +44,7 @@ module Fizzy
resource :webhooks, only: :create
end
resources :devices, only: [ :index, :create, :destroy ] do
delete :destroy, on: :collection, as: :unregister
end
resources :devices, only: [ :index, :create, :destroy ]
end
end
saas/test/controllers/devices_controller_test.rb
+13 -21
View File
@@ -6,8 +6,6 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest
sign_in_as @user
end
# === Index (Web) ===
test "index shows user devices" do
@user.devices.create!(token: "test_token_123", platform: "apple", name: "iPhone 15 Pro")
@@ -35,8 +33,6 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest
assert_response :redirect
end
# === Create (API) ===
test "creates a new device via api" do
token = SecureRandom.hex(32)
@@ -108,7 +104,7 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest
name: "Surface"
}, as: :json
assert_response :bad_request
assert_response :unprocessable_entity
end
test "rejects missing token" do
@@ -131,8 +127,6 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest
assert_response :redirect
end
# === Destroy (Web) ===
test "destroys device by id" do
device = @user.devices.create!(
token: "token_to_delete",
@@ -148,15 +142,15 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest
assert_not ActionPushNative::Device.exists?(device.id)
end
test "does nothing when device not found by id" do
test "returns not found when device not found by id" do
assert_no_difference "ActionPushNative::Device.count" do
delete device_path(id: "nonexistent")
end
assert_redirected_to devices_path
assert_response :not_found
end
test "cannot destroy another user's device by id" do
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",
@@ -168,7 +162,7 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest
delete device_path(device)
end
assert_redirected_to devices_path
assert_response :not_found
assert ActionPushNative::Device.exists?(device.id)
end
@@ -187,8 +181,6 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest
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",
@@ -197,22 +189,22 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest
)
assert_difference "ActionPushNative::Device.count", -1 do
delete unregister_devices_path, params: { token: "token_to_unregister" }, as: :json
delete device_path("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
test "returns not found when device not found by token" do
assert_no_difference "ActionPushNative::Device.count" do
delete unregister_devices_path, params: { token: "nonexistent_token" }, as: :json
delete device_path("nonexistent_token"), as: :json
end
assert_response :no_content
assert_response :not_found
end
test "cannot destroy another user's device by token" do
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",
@@ -221,10 +213,10 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest
)
assert_no_difference "ActionPushNative::Device.count" do
delete unregister_devices_path, params: { token: "other_users_token" }, as: :json
delete device_path("other_users_token"), as: :json
end
assert_response :no_content
assert_response :not_found
assert ActionPushNative::Device.exists?(device.id)
end
@@ -237,7 +229,7 @@ class DevicesControllerTest < ActionDispatch::IntegrationTest
sign_out
delete unregister_devices_path, params: { token: "my_token" }, as: :json
delete device_path("my_token"), as: :json
assert_response :redirect
assert ActionPushNative::Device.exists?(device.id)