diff --git a/app/controllers/api/web/push_subscriptions_controller.rb b/app/controllers/api/web/push_subscriptions_controller.rb index 5167928e93..167d16fc4d 100644 --- a/app/controllers/api/web/push_subscriptions_controller.rb +++ b/app/controllers/api/web/push_subscriptions_controller.rb @@ -3,45 +3,56 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController before_action :require_user! before_action :set_push_subscription, only: :update + before_action :destroy_previous_subscriptions, only: :create, if: :prior_subscriptions? + after_action :update_session_with_subscription, only: :create def create - active_session = current_session + @push_subscription = ::Web::PushSubscription.create!(web_push_subscription_params) - unless active_session.web_push_subscription.nil? - active_session.web_push_subscription.destroy! - active_session.update!(web_push_subscription: nil) - end + render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer + end - # Mobile devices do not support regular notifications, so we enable push notifications by default - alerts_enabled = active_session.detection.device.mobile? || active_session.detection.device.tablet? + def update + @push_subscription.update!(data: data_params) + render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer + end - data = { - policy: 'all', - alerts: Notification::TYPES.index_with { alerts_enabled }, - } + private - data.deep_merge!(data_params) if params[:data] + def active_session + @active_session ||= current_session + end - push_subscription = ::Web::PushSubscription.create!( - endpoint: subscription_params[:endpoint], - key_p256dh: subscription_params[:keys][:p256dh], - key_auth: subscription_params[:keys][:auth], - data: data, - user_id: active_session.user_id, - access_token_id: active_session.access_token_id - ) + def destroy_previous_subscriptions + active_session.web_push_subscription.destroy! + active_session.update!(web_push_subscription: nil) + end + + def prior_subscriptions? + active_session.web_push_subscription.present? + end - active_session.update!(web_push_subscription: push_subscription) + def subscription_data + default_subscription_data.tap do |data| + data.deep_merge!(data_params) if params[:data] + end + end - render json: push_subscription, serializer: REST::WebPushSubscriptionSerializer + def default_subscription_data + { + policy: 'all', + alerts: Notification::TYPES.index_with { alerts_enabled }, + } end - def update - @push_subscription.update!(data: data_params) - render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer + def alerts_enabled + # Mobile devices do not support regular notifications, so we enable push notifications by default + active_session.detection.device.mobile? || active_session.detection.device.tablet? end - private + def update_session_with_subscription + active_session.update!(web_push_subscription: @push_subscription) + end def set_push_subscription @push_subscription = ::Web::PushSubscription.find(params[:id]) @@ -51,6 +62,17 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController @subscription_params ||= params.require(:subscription).permit(:endpoint, keys: [:auth, :p256dh]) end + def web_push_subscription_params + { + access_token_id: active_session.access_token_id, + data: subscription_data, + endpoint: subscription_params[:endpoint], + key_auth: subscription_params[:keys][:auth], + key_p256dh: subscription_params[:keys][:p256dh], + user_id: active_session.user_id, + } + end + def data_params @data_params ||= params.require(:data).permit(:policy, alerts: Notification::TYPES) end diff --git a/spec/controllers/api/web/push_subscriptions_controller_spec.rb b/spec/controllers/api/web/push_subscriptions_controller_spec.rb index 9f027ede90..58677841ca 100644 --- a/spec/controllers/api/web/push_subscriptions_controller_spec.rb +++ b/spec/controllers/api/web/push_subscriptions_controller_spec.rb @@ -37,37 +37,49 @@ describe Api::Web::PushSubscriptionsController do } end - describe 'POST #create' do - it 'saves push subscriptions' do - sign_in(user) + before do + sign_in(user) - stub_request(:post, create_payload[:subscription][:endpoint]).to_return(status: 200) + stub_request(:post, create_payload[:subscription][:endpoint]).to_return(status: 200) + end + describe 'POST #create' do + it 'saves push subscriptions' do post :create, format: :json, params: create_payload + expect(response).to have_http_status(200) + user.reload - push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) + expect(created_push_subscription).to have_attributes( + endpoint: eq(create_payload[:subscription][:endpoint]), + key_p256dh: eq(create_payload[:subscription][:keys][:p256dh]), + key_auth: eq(create_payload[:subscription][:keys][:auth]) + ) + expect(user.session_activations.first.web_push_subscription).to eq(created_push_subscription) + end + + context 'with a user who has a session with a prior subscription' do + let!(:prior_subscription) { Fabricate(:web_push_subscription, session_activation: user.session_activations.last) } + + it 'destroys prior subscription when creating new one' do + post :create, format: :json, params: create_payload - expect(push_subscription['endpoint']).to eq(create_payload[:subscription][:endpoint]) - expect(push_subscription['key_p256dh']).to eq(create_payload[:subscription][:keys][:p256dh]) - expect(push_subscription['key_auth']).to eq(create_payload[:subscription][:keys][:auth]) + expect(response).to have_http_status(200) + expect { prior_subscription.reload }.to raise_error(ActiveRecord::RecordNotFound) + end end context 'with initial data' do it 'saves alert settings' do - sign_in(user) - - stub_request(:post, create_payload[:subscription][:endpoint]).to_return(status: 200) - post :create, format: :json, params: create_payload.merge(alerts_payload) - push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) + expect(response).to have_http_status(200) - expect(push_subscription.data['policy']).to eq 'all' + expect(created_push_subscription.data['policy']).to eq 'all' %w(follow follow_request favourite reblog mention poll status).each do |type| - expect(push_subscription.data['alerts'][type]).to eq(alerts_payload[:data][:alerts][type.to_sym].to_s) + expect(created_push_subscription.data['alerts'][type]).to eq(alerts_payload[:data][:alerts][type.to_sym].to_s) end end end @@ -75,23 +87,23 @@ describe Api::Web::PushSubscriptionsController do describe 'PUT #update' do it 'changes alert settings' do - sign_in(user) - - stub_request(:post, create_payload[:subscription][:endpoint]).to_return(status: 200) - post :create, format: :json, params: create_payload - alerts_payload[:id] = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]).id + expect(response).to have_http_status(200) - put :update, format: :json, params: alerts_payload + alerts_payload[:id] = created_push_subscription.id - push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) + put :update, format: :json, params: alerts_payload - expect(push_subscription.data['policy']).to eq 'all' + expect(created_push_subscription.data['policy']).to eq 'all' %w(follow follow_request favourite reblog mention poll status).each do |type| - expect(push_subscription.data['alerts'][type]).to eq(alerts_payload[:data][:alerts][type.to_sym].to_s) + expect(created_push_subscription.data['alerts'][type]).to eq(alerts_payload[:data][:alerts][type.to_sym].to_s) end end end + + def created_push_subscription + Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) + end end