Add delay to profile updates to debounce them (#34137)
This commit is contained in:
		
					parent
					
						
							
								725a68d273
							
						
					
				
			
			
				commit
				
					
						420ffdfb62
					
				
			
		
					 12 changed files with 17 additions and 23 deletions
				
			
		|  | @ -14,7 +14,7 @@ class Api::V1::Accounts::CredentialsController < Api::BaseController | ||||||
|     @account = current_account |     @account = current_account | ||||||
|     UpdateAccountService.new.call(@account, account_params, raise_error: true) |     UpdateAccountService.new.call(@account, account_params, raise_error: true) | ||||||
|     current_user.update(user_params) if user_params |     current_user.update(user_params) if user_params | ||||||
|     ActivityPub::UpdateDistributionWorker.perform_async(@account.id) |     ActivityPub::UpdateDistributionWorker.perform_in(ActivityPub::UpdateDistributionWorker::DEBOUNCE_DELAY, @account.id) | ||||||
|     render json: @account, serializer: REST::CredentialAccountSerializer |     render json: @account, serializer: REST::CredentialAccountSerializer | ||||||
|   rescue ActiveRecord::RecordInvalid => e |   rescue ActiveRecord::RecordInvalid => e | ||||||
|     render json: ValidationErrorFormatter.new(e).as_json, status: 422 |     render json: ValidationErrorFormatter.new(e).as_json, status: 422 | ||||||
|  |  | ||||||
|  | @ -7,7 +7,7 @@ class Api::V1::Profile::AvatarsController < Api::BaseController | ||||||
|   def destroy |   def destroy | ||||||
|     @account = current_account |     @account = current_account | ||||||
|     UpdateAccountService.new.call(@account, { avatar: nil }, raise_error: true) |     UpdateAccountService.new.call(@account, { avatar: nil }, raise_error: true) | ||||||
|     ActivityPub::UpdateDistributionWorker.perform_async(@account.id) |     ActivityPub::UpdateDistributionWorker.perform_in(ActivityPub::UpdateDistributionWorker::DEBOUNCE_DELAY, @account.id) | ||||||
|     render json: @account, serializer: REST::CredentialAccountSerializer |     render json: @account, serializer: REST::CredentialAccountSerializer | ||||||
|   end |   end | ||||||
| end | end | ||||||
|  |  | ||||||
|  | @ -7,7 +7,7 @@ class Api::V1::Profile::HeadersController < Api::BaseController | ||||||
|   def destroy |   def destroy | ||||||
|     @account = current_account |     @account = current_account | ||||||
|     UpdateAccountService.new.call(@account, { header: nil }, raise_error: true) |     UpdateAccountService.new.call(@account, { header: nil }, raise_error: true) | ||||||
|     ActivityPub::UpdateDistributionWorker.perform_async(@account.id) |     ActivityPub::UpdateDistributionWorker.perform_in(ActivityPub::UpdateDistributionWorker::DEBOUNCE_DELAY, @account.id) | ||||||
|     render json: @account, serializer: REST::CredentialAccountSerializer |     render json: @account, serializer: REST::CredentialAccountSerializer | ||||||
|   end |   end | ||||||
| end | end | ||||||
|  |  | ||||||
|  | @ -8,7 +8,7 @@ module Settings | ||||||
|     def destroy |     def destroy | ||||||
|       if valid_picture? |       if valid_picture? | ||||||
|         if UpdateAccountService.new.call(@account, { @picture => nil, "#{@picture}_remote_url" => '' }) |         if UpdateAccountService.new.call(@account, { @picture => nil, "#{@picture}_remote_url" => '' }) | ||||||
|           ActivityPub::UpdateDistributionWorker.perform_async(@account.id) |           ActivityPub::UpdateDistributionWorker.perform_in(ActivityPub::UpdateDistributionWorker::DEBOUNCE_DELAY, @account.id) | ||||||
|           redirect_to settings_profile_path, notice: I18n.t('generic.changes_saved_msg'), status: 303 |           redirect_to settings_profile_path, notice: I18n.t('generic.changes_saved_msg'), status: 303 | ||||||
|         else |         else | ||||||
|           redirect_to settings_profile_path |           redirect_to settings_profile_path | ||||||
|  |  | ||||||
|  | @ -8,7 +8,7 @@ class Settings::PrivacyController < Settings::BaseController | ||||||
|   def update |   def update | ||||||
|     if UpdateAccountService.new.call(@account, account_params.except(:settings)) |     if UpdateAccountService.new.call(@account, account_params.except(:settings)) | ||||||
|       current_user.update!(settings_attributes: account_params[:settings]) |       current_user.update!(settings_attributes: account_params[:settings]) | ||||||
|       ActivityPub::UpdateDistributionWorker.perform_async(@account.id) |       ActivityPub::UpdateDistributionWorker.perform_in(ActivityPub::UpdateDistributionWorker::DEBOUNCE_DELAY, @account.id) | ||||||
|       redirect_to settings_privacy_path, notice: I18n.t('generic.changes_saved_msg') |       redirect_to settings_privacy_path, notice: I18n.t('generic.changes_saved_msg') | ||||||
|     else |     else | ||||||
|       render :show |       render :show | ||||||
|  |  | ||||||
|  | @ -9,7 +9,7 @@ class Settings::ProfilesController < Settings::BaseController | ||||||
| 
 | 
 | ||||||
|   def update |   def update | ||||||
|     if UpdateAccountService.new.call(@account, account_params) |     if UpdateAccountService.new.call(@account, account_params) | ||||||
|       ActivityPub::UpdateDistributionWorker.perform_async(@account.id) |       ActivityPub::UpdateDistributionWorker.perform_in(ActivityPub::UpdateDistributionWorker::DEBOUNCE_DELAY, @account.id) | ||||||
|       redirect_to settings_profile_path, notice: I18n.t('generic.changes_saved_msg') |       redirect_to settings_profile_path, notice: I18n.t('generic.changes_saved_msg') | ||||||
|     else |     else | ||||||
|       @account.build_fields |       @account.build_fields | ||||||
|  |  | ||||||
|  | @ -8,7 +8,7 @@ class Settings::VerificationsController < Settings::BaseController | ||||||
| 
 | 
 | ||||||
|   def update |   def update | ||||||
|     if UpdateAccountService.new.call(@account, account_params) |     if UpdateAccountService.new.call(@account, account_params) | ||||||
|       ActivityPub::UpdateDistributionWorker.perform_async(@account.id) |       ActivityPub::UpdateDistributionWorker.perform_in(ActivityPub::UpdateDistributionWorker::DEBOUNCE_DELAY, @account.id) | ||||||
|       redirect_to settings_verification_path, notice: I18n.t('generic.changes_saved_msg') |       redirect_to settings_verification_path, notice: I18n.t('generic.changes_saved_msg') | ||||||
|     else |     else | ||||||
|       render :show |       render :show | ||||||
|  |  | ||||||
|  | @ -1,6 +1,8 @@ | ||||||
| # frozen_string_literal: true | # frozen_string_literal: true | ||||||
| 
 | 
 | ||||||
| class ActivityPub::UpdateDistributionWorker < ActivityPub::RawDistributionWorker | class ActivityPub::UpdateDistributionWorker < ActivityPub::RawDistributionWorker | ||||||
|  |   DEBOUNCE_DELAY = 5.seconds | ||||||
|  | 
 | ||||||
|   sidekiq_options queue: 'push', lock: :until_executed, lock_ttl: 1.day.to_i |   sidekiq_options queue: 'push', lock: :until_executed, lock_ttl: 1.day.to_i | ||||||
| 
 | 
 | ||||||
|   # Distribute an profile update to servers that might have a copy |   # Distribute an profile update to servers that might have a copy | ||||||
|  |  | ||||||
|  | @ -53,8 +53,6 @@ RSpec.describe 'credentials API' do | ||||||
|       patch '/api/v1/accounts/update_credentials', headers: headers, params: params |       patch '/api/v1/accounts/update_credentials', headers: headers, params: params | ||||||
|     end |     end | ||||||
| 
 | 
 | ||||||
|     before { allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_async) } |  | ||||||
| 
 |  | ||||||
|     let(:params) do |     let(:params) do | ||||||
|       { |       { | ||||||
|         avatar: fixture_file_upload('avatar.gif', 'image/gif'), |         avatar: fixture_file_upload('avatar.gif', 'image/gif'), | ||||||
|  | @ -113,7 +111,7 @@ RSpec.describe 'credentials API' do | ||||||
|       }) |       }) | ||||||
| 
 | 
 | ||||||
|       expect(ActivityPub::UpdateDistributionWorker) |       expect(ActivityPub::UpdateDistributionWorker) | ||||||
|         .to have_received(:perform_async).with(user.account_id) |         .to have_enqueued_sidekiq_job(user.account_id) | ||||||
|     end |     end | ||||||
| 
 | 
 | ||||||
|     def expect_account_updates |     def expect_account_updates | ||||||
|  |  | ||||||
|  | @ -15,10 +15,6 @@ RSpec.describe 'Deleting profile images' do | ||||||
|   let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } |   let(:headers) { { 'Authorization' => "Bearer #{token.token}" } } | ||||||
| 
 | 
 | ||||||
|   describe 'DELETE /api/v1/profile' do |   describe 'DELETE /api/v1/profile' do | ||||||
|     before do |  | ||||||
|       allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_async) |  | ||||||
|     end |  | ||||||
| 
 |  | ||||||
|     context 'when deleting an avatar' do |     context 'when deleting an avatar' do | ||||||
|       context 'with wrong scope' do |       context 'with wrong scope' do | ||||||
|         before do |         before do | ||||||
|  | @ -38,7 +34,8 @@ RSpec.describe 'Deleting profile images' do | ||||||
|         account.reload |         account.reload | ||||||
|         expect(account.avatar).to_not exist |         expect(account.avatar).to_not exist | ||||||
|         expect(account.header).to exist |         expect(account.header).to exist | ||||||
|         expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_async).with(account.id) |         expect(ActivityPub::UpdateDistributionWorker) | ||||||
|  |           .to have_enqueued_sidekiq_job(account.id) | ||||||
|       end |       end | ||||||
|     end |     end | ||||||
| 
 | 
 | ||||||
|  | @ -61,7 +58,8 @@ RSpec.describe 'Deleting profile images' do | ||||||
|         account.reload |         account.reload | ||||||
|         expect(account.avatar).to exist |         expect(account.avatar).to exist | ||||||
|         expect(account.header).to_not exist |         expect(account.header).to_not exist | ||||||
|         expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_async).with(account.id) |         expect(ActivityPub::UpdateDistributionWorker) | ||||||
|  |           .to have_enqueued_sidekiq_job(account.id) | ||||||
|       end |       end | ||||||
|     end |     end | ||||||
|   end |   end | ||||||
|  |  | ||||||
|  | @ -11,8 +11,6 @@ RSpec.describe 'Settings Privacy' do | ||||||
|     before { user.account.update(discoverable: false) } |     before { user.account.update(discoverable: false) } | ||||||
| 
 | 
 | ||||||
|     context 'with a successful update' do |     context 'with a successful update' do | ||||||
|       before { allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_async) } |  | ||||||
| 
 |  | ||||||
|       it 'updates user profile information' do |       it 'updates user profile information' do | ||||||
|         # View settings page |         # View settings page | ||||||
|         visit settings_privacy_path |         visit settings_privacy_path | ||||||
|  | @ -29,14 +27,13 @@ RSpec.describe 'Settings Privacy' do | ||||||
|           .to have_content(I18n.t('privacy.title')) |           .to have_content(I18n.t('privacy.title')) | ||||||
|           .and have_content(success_message) |           .and have_content(success_message) | ||||||
|         expect(ActivityPub::UpdateDistributionWorker) |         expect(ActivityPub::UpdateDistributionWorker) | ||||||
|           .to have_received(:perform_async).with(user.account.id) |           .to have_enqueued_sidekiq_job(user.account.id) | ||||||
|       end |       end | ||||||
|     end |     end | ||||||
| 
 | 
 | ||||||
|     context 'with a failed update' do |     context 'with a failed update' do | ||||||
|       before do |       before do | ||||||
|         allow(UpdateAccountService).to receive(:new).and_return(failing_update_service) |         allow(UpdateAccountService).to receive(:new).and_return(failing_update_service) | ||||||
|         allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_async) |  | ||||||
|       end |       end | ||||||
| 
 | 
 | ||||||
|       it 'updates user profile information' do |       it 'updates user profile information' do | ||||||
|  | @ -54,7 +51,7 @@ RSpec.describe 'Settings Privacy' do | ||||||
|         expect(page) |         expect(page) | ||||||
|           .to have_content(I18n.t('privacy.title')) |           .to have_content(I18n.t('privacy.title')) | ||||||
|         expect(ActivityPub::UpdateDistributionWorker) |         expect(ActivityPub::UpdateDistributionWorker) | ||||||
|           .to_not have_received(:perform_async) |           .to_not have_enqueued_sidekiq_job(anything) | ||||||
|       end |       end | ||||||
| 
 | 
 | ||||||
|       private |       private | ||||||
|  |  | ||||||
|  | @ -7,7 +7,6 @@ RSpec.describe 'Settings profile page' do | ||||||
|   let(:account) { user.account } |   let(:account) { user.account } | ||||||
| 
 | 
 | ||||||
|   before do |   before do | ||||||
|     allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_async) |  | ||||||
|     sign_in user |     sign_in user | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|  | @ -24,7 +23,7 @@ RSpec.describe 'Settings profile page' do | ||||||
|       .to change { account.reload.display_name }.to('New name') |       .to change { account.reload.display_name }.to('New name') | ||||||
|       .and(change { account.reload.avatar.instance.avatar_file_name }.from(nil).to(be_present)) |       .and(change { account.reload.avatar.instance.avatar_file_name }.from(nil).to(be_present)) | ||||||
|     expect(ActivityPub::UpdateDistributionWorker) |     expect(ActivityPub::UpdateDistributionWorker) | ||||||
|       .to have_received(:perform_async).with(account.id) |       .to have_enqueued_sidekiq_job(account.id) | ||||||
|   end |   end | ||||||
| 
 | 
 | ||||||
|   def display_name_field |   def display_name_field | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue