Add policy param to POST /api/v1/push/subscriptions (#16040)
				
					
				
			With possible values `all`, `followed`, `follower`, and `none`, control from whom notifications will generate a Web Push alert
This commit is contained in:
		
							parent
							
								
									0778ebc0b9
								
							
						
					
					
						commit
						2cab9c9f06
					
				
					 6 changed files with 170 additions and 51 deletions
				
			
		|  | @ -3,13 +3,13 @@ | |||
| class Api::V1::Push::SubscriptionsController < Api::BaseController | ||||
|   before_action -> { doorkeeper_authorize! :push } | ||||
|   before_action :require_user! | ||||
|   before_action :set_web_push_subscription | ||||
|   before_action :check_web_push_subscription, only: [:show, :update] | ||||
|   before_action :set_push_subscription | ||||
|   before_action :check_push_subscription, only: [:show, :update] | ||||
| 
 | ||||
|   def create | ||||
|     @web_subscription&.destroy! | ||||
|     @push_subscription&.destroy! | ||||
| 
 | ||||
|     @web_subscription = ::Web::PushSubscription.create!( | ||||
|     @push_subscription = Web::PushSubscription.create!( | ||||
|       endpoint: subscription_params[:endpoint], | ||||
|       key_p256dh: subscription_params[:keys][:p256dh], | ||||
|       key_auth: subscription_params[:keys][:auth], | ||||
|  | @ -18,31 +18,31 @@ class Api::V1::Push::SubscriptionsController < Api::BaseController | |||
|       access_token_id: doorkeeper_token.id | ||||
|     ) | ||||
| 
 | ||||
|     render json: @web_subscription, serializer: REST::WebPushSubscriptionSerializer | ||||
|     render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer | ||||
|   end | ||||
| 
 | ||||
|   def show | ||||
|     render json: @web_subscription, serializer: REST::WebPushSubscriptionSerializer | ||||
|     render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer | ||||
|   end | ||||
| 
 | ||||
|   def update | ||||
|     @web_subscription.update!(data: data_params) | ||||
|     render json: @web_subscription, serializer: REST::WebPushSubscriptionSerializer | ||||
|     @push_subscription.update!(data: data_params) | ||||
|     render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer | ||||
|   end | ||||
| 
 | ||||
|   def destroy | ||||
|     @web_subscription&.destroy! | ||||
|     @push_subscription&.destroy! | ||||
|     render_empty | ||||
|   end | ||||
| 
 | ||||
|   private | ||||
| 
 | ||||
|   def set_web_push_subscription | ||||
|     @web_subscription = ::Web::PushSubscription.find_by(access_token_id: doorkeeper_token.id) | ||||
|   def set_push_subscription | ||||
|     @push_subscription = Web::PushSubscription.find_by(access_token_id: doorkeeper_token.id) | ||||
|   end | ||||
| 
 | ||||
|   def check_web_push_subscription | ||||
|     not_found if @web_subscription.nil? | ||||
|   def check_push_subscription | ||||
|     not_found if @push_subscription.nil? | ||||
|   end | ||||
| 
 | ||||
|   def subscription_params | ||||
|  | @ -52,6 +52,6 @@ class Api::V1::Push::SubscriptionsController < Api::BaseController | |||
|   def data_params | ||||
|     return {} if params[:data].blank? | ||||
| 
 | ||||
|     params.require(:data).permit(alerts: [:follow, :follow_request, :favourite, :reblog, :mention, :poll, :status]) | ||||
|     params.require(:data).permit(:policy, alerts: [:follow, :follow_request, :favourite, :reblog, :mention, :poll, :status]) | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -2,6 +2,7 @@ | |||
| 
 | ||||
| class Api::Web::PushSubscriptionsController < Api::Web::BaseController | ||||
|   before_action :require_user! | ||||
|   before_action :set_push_subscription, only: :update | ||||
| 
 | ||||
|   def create | ||||
|     active_session = current_session | ||||
|  | @ -15,9 +16,11 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController | |||
|     alerts_enabled = active_session.detection.device.mobile? || active_session.detection.device.tablet? | ||||
| 
 | ||||
|     data = { | ||||
|       policy: 'all', | ||||
| 
 | ||||
|       alerts: { | ||||
|         follow: alerts_enabled, | ||||
|         follow_request: false, | ||||
|         follow_request: alerts_enabled, | ||||
|         favourite: alerts_enabled, | ||||
|         reblog: alerts_enabled, | ||||
|         mention: alerts_enabled, | ||||
|  | @ -28,7 +31,7 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController | |||
| 
 | ||||
|     data.deep_merge!(data_params) if params[:data] | ||||
| 
 | ||||
|     web_subscription = ::Web::PushSubscription.create!( | ||||
|     push_subscription = ::Web::PushSubscription.create!( | ||||
|       endpoint: subscription_params[:endpoint], | ||||
|       key_p256dh: subscription_params[:keys][:p256dh], | ||||
|       key_auth: subscription_params[:keys][:auth], | ||||
|  | @ -37,27 +40,27 @@ class Api::Web::PushSubscriptionsController < Api::Web::BaseController | |||
|       access_token_id: active_session.access_token_id | ||||
|     ) | ||||
| 
 | ||||
|     active_session.update!(web_push_subscription: web_subscription) | ||||
|     active_session.update!(web_push_subscription: push_subscription) | ||||
| 
 | ||||
|     render json: web_subscription, serializer: REST::WebPushSubscriptionSerializer | ||||
|     render json: push_subscription, serializer: REST::WebPushSubscriptionSerializer | ||||
|   end | ||||
| 
 | ||||
|   def update | ||||
|     params.require([:id]) | ||||
| 
 | ||||
|     web_subscription = ::Web::PushSubscription.find(params[:id]) | ||||
|     web_subscription.update!(data: data_params) | ||||
| 
 | ||||
|     render json: web_subscription, serializer: REST::WebPushSubscriptionSerializer | ||||
|     @push_subscription.update!(data: data_params) | ||||
|     render json: @push_subscription, serializer: REST::WebPushSubscriptionSerializer | ||||
|   end | ||||
| 
 | ||||
|   private | ||||
| 
 | ||||
|   def set_push_subscription | ||||
|     @push_subscription = ::Web::PushSubscription.find(params[:id]) | ||||
|   end | ||||
| 
 | ||||
|   def subscription_params | ||||
|     @subscription_params ||= params.require(:subscription).permit(:endpoint, keys: [:auth, :p256dh]) | ||||
|   end | ||||
| 
 | ||||
|   def data_params | ||||
|     @data_params ||= params.require(:data).permit(alerts: [:follow, :follow_request, :favourite, :reblog, :mention, :poll, :status]) | ||||
|     @data_params ||= params.require(:data).permit(:policy, alerts: [:follow, :follow_request, :favourite, :reblog, :mention, :poll, :status]) | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -47,7 +47,7 @@ class Web::PushSubscription < ApplicationRecord | |||
|   end | ||||
| 
 | ||||
|   def pushable?(notification) | ||||
|     ActiveModel::Type::Boolean.new.cast(data&.dig('alerts', notification.type.to_s)) | ||||
|     policy_allows_notification?(notification) && alert_enabled_for_notification_type?(notification) | ||||
|   end | ||||
| 
 | ||||
|   def associated_user | ||||
|  | @ -100,4 +100,25 @@ class Web::PushSubscription < ApplicationRecord | |||
|   def contact_email | ||||
|     @contact_email ||= ::Setting.site_contact_email | ||||
|   end | ||||
| 
 | ||||
|   def alert_enabled_for_notification_type?(notification) | ||||
|     truthy?(data&.dig('alerts', notification.type.to_s)) | ||||
|   end | ||||
| 
 | ||||
|   def policy_allows_notification?(notification) | ||||
|     case data&.dig('policy') | ||||
|     when nil, 'all' | ||||
|       true | ||||
|     when 'none' | ||||
|       false | ||||
|     when 'followed' | ||||
|       notification.account.following?(notification.from_account) | ||||
|     when 'follower' | ||||
|       notification.from_account.following?(notification.account) | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|   def truthy?(val) | ||||
|     ActiveModel::Type::Boolean.new.cast(val) | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -27,20 +27,27 @@ describe Api::V1::Push::SubscriptionsController do | |||
|   let(:alerts_payload) do | ||||
|     { | ||||
|       data: { | ||||
|         policy: 'all', | ||||
| 
 | ||||
|         alerts: { | ||||
|           follow: true, | ||||
|           follow_request: true, | ||||
|           favourite: false, | ||||
|           reblog: true, | ||||
|           mention: false, | ||||
|           poll: true, | ||||
|           status: false, | ||||
|         } | ||||
|       } | ||||
|     }.with_indifferent_access | ||||
|   end | ||||
| 
 | ||||
|   describe 'POST #create' do | ||||
|     it 'saves push subscriptions' do | ||||
|     before do | ||||
|       post :create, params: create_payload | ||||
|     end | ||||
| 
 | ||||
|     it 'saves push subscriptions' do | ||||
|       push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) | ||||
| 
 | ||||
|       expect(push_subscription.endpoint).to eq(create_payload[:subscription][:endpoint]) | ||||
|  | @ -52,31 +59,34 @@ describe Api::V1::Push::SubscriptionsController do | |||
| 
 | ||||
|     it 'replaces old subscription on repeat calls' do | ||||
|       post :create, params: create_payload | ||||
|       post :create, params: create_payload | ||||
| 
 | ||||
|       expect(Web::PushSubscription.where(endpoint: create_payload[:subscription][:endpoint]).count).to eq 1 | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|   describe 'PUT #update' do | ||||
|     it 'changes alert settings' do | ||||
|     before do | ||||
|       post :create, params: create_payload | ||||
|       put :update, params: alerts_payload | ||||
|     end | ||||
| 
 | ||||
|     it 'changes alert settings' do | ||||
|       push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) | ||||
| 
 | ||||
|       expect(push_subscription.data.dig('alerts', 'follow')).to eq(alerts_payload[:data][:alerts][:follow].to_s) | ||||
|       expect(push_subscription.data.dig('alerts', 'favourite')).to eq(alerts_payload[:data][:alerts][:favourite].to_s) | ||||
|       expect(push_subscription.data.dig('alerts', 'reblog')).to eq(alerts_payload[:data][:alerts][:reblog].to_s) | ||||
|       expect(push_subscription.data.dig('alerts', 'mention')).to eq(alerts_payload[:data][:alerts][:mention].to_s) | ||||
|       expect(push_subscription.data['policy']).to eq(alerts_payload[:data][:policy]) | ||||
| 
 | ||||
|       %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) | ||||
|       end | ||||
|     end | ||||
|   end | ||||
| 
 | ||||
|   describe 'DELETE #destroy' do | ||||
|     it 'removes the subscription' do | ||||
|     before do | ||||
|       post :create, params: create_payload | ||||
|       delete :destroy | ||||
|     end | ||||
| 
 | ||||
|     it 'removes the subscription' do | ||||
|       expect(Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint])).to be_nil | ||||
|     end | ||||
|   end | ||||
|  |  | |||
|  | @ -22,11 +22,16 @@ describe Api::Web::PushSubscriptionsController do | |||
|   let(:alerts_payload) do | ||||
|     { | ||||
|       data: { | ||||
|         policy: 'all', | ||||
| 
 | ||||
|         alerts: { | ||||
|           follow: true, | ||||
|           follow_request: false, | ||||
|           favourite: false, | ||||
|           reblog: true, | ||||
|           mention: false, | ||||
|           poll: true, | ||||
|           status: false, | ||||
|         } | ||||
|       } | ||||
|     } | ||||
|  | @ -59,10 +64,11 @@ describe Api::Web::PushSubscriptionsController do | |||
| 
 | ||||
|         push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) | ||||
| 
 | ||||
|         expect(push_subscription.data['alerts']['follow']).to eq(alerts_payload[:data][:alerts][:follow].to_s) | ||||
|         expect(push_subscription.data['alerts']['favourite']).to eq(alerts_payload[:data][:alerts][:favourite].to_s) | ||||
|         expect(push_subscription.data['alerts']['reblog']).to eq(alerts_payload[:data][:alerts][:reblog].to_s) | ||||
|         expect(push_subscription.data['alerts']['mention']).to eq(alerts_payload[:data][:alerts][:mention].to_s) | ||||
|         expect(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) | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|   end | ||||
|  | @ -81,10 +87,11 @@ describe Api::Web::PushSubscriptionsController do | |||
| 
 | ||||
|       push_subscription = Web::PushSubscription.find_by(endpoint: create_payload[:subscription][:endpoint]) | ||||
| 
 | ||||
|       expect(push_subscription.data['alerts']['follow']).to eq(alerts_payload[:data][:alerts][:follow].to_s) | ||||
|       expect(push_subscription.data['alerts']['favourite']).to eq(alerts_payload[:data][:alerts][:favourite].to_s) | ||||
|       expect(push_subscription.data['alerts']['reblog']).to eq(alerts_payload[:data][:alerts][:reblog].to_s) | ||||
|       expect(push_subscription.data['alerts']['mention']).to eq(alerts_payload[:data][:alerts][:mention].to_s) | ||||
|       expect(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) | ||||
|       end | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
|  | @ -1,16 +1,94 @@ | |||
| require 'rails_helper' | ||||
| 
 | ||||
| RSpec.describe Web::PushSubscription, type: :model do | ||||
|   let(:alerts) { { mention: true, reblog: false, follow: true, follow_request: false, favourite: true } } | ||||
|   let(:push_subscription) { Web::PushSubscription.new(data: { alerts: alerts }) } | ||||
|   let(:account) { Fabricate(:account) } | ||||
| 
 | ||||
|   let(:policy) { 'all' } | ||||
| 
 | ||||
|   let(:data) do | ||||
|     { | ||||
|       policy: policy, | ||||
| 
 | ||||
|       alerts: { | ||||
|         mention: true, | ||||
|         reblog: false, | ||||
|         follow: true, | ||||
|         follow_request: false, | ||||
|         favourite: true, | ||||
|       }, | ||||
|     } | ||||
|   end | ||||
| 
 | ||||
|   subject { described_class.new(data: data) } | ||||
| 
 | ||||
|   describe '#pushable?' do | ||||
|     it 'obeys alert settings' do | ||||
|       expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'Mention'))).to eq true | ||||
|       expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'Status'))).to eq false | ||||
|       expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'Follow'))).to eq true | ||||
|       expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'FollowRequest'))).to eq false | ||||
|       expect(push_subscription.send(:pushable?, Notification.new(activity_type: 'Favourite'))).to eq true | ||||
|     let(:notification_type) { :mention } | ||||
|     let(:notification) { Fabricate(:notification, account: account, type: notification_type) } | ||||
| 
 | ||||
|     %i(mention reblog follow follow_request favourite).each do |type| | ||||
|       context "when notification is a #{type}" do | ||||
|         let(:notification_type) { type } | ||||
| 
 | ||||
|         it "returns boolean corresonding to alert setting" do | ||||
|           expect(subject.pushable?(notification)).to eq data[:alerts][type] | ||||
|         end | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     context 'when policy is all' do | ||||
|       let(:policy) { 'all' } | ||||
| 
 | ||||
|       it 'returns true' do | ||||
|         expect(subject.pushable?(notification)).to eq true | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     context 'when policy is none' do | ||||
|       let(:policy) { 'none' } | ||||
| 
 | ||||
|       it 'returns false' do | ||||
|         expect(subject.pushable?(notification)).to eq false | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     context 'when policy is followed' do | ||||
|       let(:policy) { 'followed' } | ||||
| 
 | ||||
|       context 'and notification is from someone you follow' do | ||||
|         before do | ||||
|           account.follow!(notification.from_account) | ||||
|         end | ||||
| 
 | ||||
|         it 'returns true' do | ||||
|           expect(subject.pushable?(notification)).to eq true | ||||
|         end | ||||
|       end | ||||
| 
 | ||||
|       context 'and notification is not from someone you follow' do | ||||
|         it 'returns false' do | ||||
|           expect(subject.pushable?(notification)).to eq false | ||||
|         end | ||||
|       end | ||||
|     end | ||||
| 
 | ||||
|     context 'when policy is follower' do | ||||
|       let(:policy) { 'follower' } | ||||
| 
 | ||||
|       context 'and notification is from someone who follows you' do | ||||
|         before do | ||||
|           notification.from_account.follow!(account) | ||||
|         end | ||||
| 
 | ||||
|         it 'returns true' do | ||||
|           expect(subject.pushable?(notification)).to eq true | ||||
|         end | ||||
|       end | ||||
| 
 | ||||
|       context 'and notification is not from someone who follows you' do | ||||
|         it 'returns false' do | ||||
|           expect(subject.pushable?(notification)).to eq false | ||||
|         end | ||||
|       end | ||||
|     end | ||||
|   end | ||||
| end | ||||
|  |  | |||
		Loading…
	
		Reference in a new issue