Add delay to profile updates to debounce them (#34137)

app/controllers/api/v1/accounts/credentials_controller.rb

@@ -14,7 +14,7 @@ class Api::V1::Accounts::CredentialsController < Api::BaseController
     @account = current_account
     UpdateAccountService.new.call(@account, account_params, raise_error: true)
     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
   end
 

app/controllers/api/v1/profile/avatars_controller.rb

@@ -7,7 +7,7 @@ class Api::V1::Profile::AvatarsController < Api::BaseController
   def destroy
     @account = current_account
     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
   end
 end

app/controllers/api/v1/profile/headers_controller.rb

@@ -7,7 +7,7 @@ class Api::V1::Profile::HeadersController < Api::BaseController
   def destroy
     @account = current_account
     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
   end
 end

app/controllers/settings/pictures_controller.rb

@@ -8,7 +8,7 @@ module Settings
     def destroy
       if valid_picture?
         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
         else
           redirect_to settings_profile_path

app/controllers/settings/privacy_controller.rb

@@ -8,7 +8,7 @@ class Settings::PrivacyController < Settings::BaseController
   def update
     if UpdateAccountService.new.call(@account, account_params.except(: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')
     else
       render :show

app/controllers/settings/profiles_controller.rb

@@ -9,7 +9,7 @@ class Settings::ProfilesController < Settings::BaseController
 
   def update
     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')
     else
       @account.build_fields

app/workers/activitypub/update_distribution_worker.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class ActivityPub::UpdateDistributionWorker < ActivityPub::RawDistributionWorker
+  DEBOUNCE_DELAY = 5.seconds
+
   sidekiq_options queue: 'push', lock: :until_executed, lock_ttl: 1.day.to_i
 
   # Distribute an profile update to servers that might have a copy

spec/controllers/api/v1/accounts/credentials_controller_spec.rb

@@ -27,7 +27,7 @@ describe Api::V1::Accounts::CredentialsController do
 
       describe 'with valid data' do
         before do
-          allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_async)
+          allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_in)
 
           patch :update, params: {
             display_name: "Alice Isn't Dead",
@@ -58,7 +58,7 @@ describe Api::V1::Accounts::CredentialsController do
         end
 
         it 'queues up an account update distribution' do
-          expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_async).with(user.account_id)
+          expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_in).with(anything, user.account_id)
         end
       end
 

spec/controllers/settings/privacy_controller_spec.rb

@@ -0,0 +1,70 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe Settings::PrivacyController do
+  render_views
+
+  let!(:user) { Fabricate(:user) }
+  let(:account) { user.account }
+
+  before do
+    sign_in user, scope: :user
+  end
+
+  describe 'GET #show' do
+    before do
+      get :show
+    end
+
+    it 'returns http success with private cache control headers', :aggregate_failures do
+      expect(response)
+        .to have_http_status(200)
+      expect(response.headers['Cache-Control']).to include('private, no-store')
+    end
+  end
+
+  describe 'PUT #update' do
+    context 'when update succeeds' do
+      before do
+        allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_in)
+      end
+
+      it 'updates the user profile' do
+        put :update, params: { account: { discoverable: '1', settings: { indexable: '1' } } }
+
+        expect(account.reload.discoverable)
+          .to be(true)
+
+        expect(response)
+          .to redirect_to(settings_privacy_path)
+
+        expect(ActivityPub::UpdateDistributionWorker)
+          .to have_received(:perform_in).with(anything, account.id)
+      end
+    end
+
+    context 'when update fails' do
+      before do
+        allow(UpdateAccountService).to receive(:new).and_return(failing_update_service)
+        allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_in)
+      end
+
+      it 'updates the user profile' do
+        put :update, params: { account: { discoverable: '1', settings: { indexable: '1' } } }
+
+        expect(response)
+          .to render_template(:show)
+
+        expect(ActivityPub::UpdateDistributionWorker)
+          .to_not have_received(:perform_in)
+      end
+
+      private
+
+      def failing_update_service
+        instance_double(UpdateAccountService, call: false)
+      end
+    end
+  end
+end

spec/controllers/settings/profiles_controller_spec.rb

@@ -32,23 +32,23 @@ RSpec.describe Settings::ProfilesController do
     end
 
     it 'updates the user profile' do
-      allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_async)
+      allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_in)
       put :update, params: { account: { display_name: 'New name' } }
       expect(account.reload.display_name).to eq 'New name'
       expect(response).to redirect_to(settings_profile_path)
-      expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_async).with(account.id)
+      expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_in).with(anything, account.id)
     end
   end
 
   describe 'PUT #update with new profile image' do
     it 'updates profile image' do
-      allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_async)
+      allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_in)
       expect(account.avatar.instance.avatar_file_name).to be_nil
 
       put :update, params: { account: { avatar: fixture_file_upload('avatar.gif', 'image/gif') } }
       expect(response).to redirect_to(settings_profile_path)
       expect(account.reload.avatar.instance.avatar_file_name).to_not be_nil
-      expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_async).with(account.id)
+      expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_in).with(anything, account.id)
     end
   end
 end

spec/requests/api/v1/accounts/credentials_spec.rb

@@ -39,7 +39,25 @@ RSpec.describe 'credentials API' do
       patch '/api/v1/accounts/update_credentials', headers: headers, params: params
     end
 
-    let(:params) { { discoverable: true, locked: false, indexable: true } }
+    before do
+      allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_in)
+    end
+
+    let(:params) do
+      {
+        avatar: fixture_file_upload('avatar.gif', 'image/gif'),
+        discoverable: true,
+        display_name: "Alice Isn't Dead",
+        header: fixture_file_upload('attachment.jpg', 'image/jpeg'),
+        indexable: true,
+        locked: false,
+        note: 'Hello!',
+        source: {
+          privacy: 'unlisted',
+          sensitive: true,
+        },
+      }
+    end
 
     it_behaves_like 'forbidden for wrong scope', 'read read:accounts'
 
@@ -59,6 +77,27 @@ RSpec.describe 'credentials API' do
         }),
         locked: false,
       })
+
+      expect(ActivityPub::UpdateDistributionWorker)
+        .to have_received(:perform_in).with(anything, user.account_id)
+    end
+
+    def expect_account_updates
+      expect(user.account.reload)
+        .to have_attributes(
+          display_name: eq("Alice Isn't Dead"),
+          note: 'Hello!',
+          avatar: exist,
+          header: exist
+        )
+    end
+
+    def expect_user_updates
+      expect(user.reload)
+        .to have_attributes(
+          setting_default_privacy: eq('unlisted'),
+          setting_default_sensitive: be(true)
+        )
     end
   end
 end

spec/requests/api/v1/profiles_spec.rb

@@ -16,7 +16,7 @@ RSpec.describe 'Deleting profile images' do
 
   describe 'DELETE /api/v1/profile' do
     before do
-      allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_async)
+      allow(ActivityPub::UpdateDistributionWorker).to receive(:perform_in)
     end
 
     context 'when deleting an avatar' do
@@ -48,12 +48,7 @@ RSpec.describe 'Deleting profile images' do
         account.reload
 
         expect(account.header).to exist
-      end
-
-      it 'queues up an account update distribution' do
-        delete '/api/v1/profile/avatar', headers: headers
-
-        expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_async).with(account.id)
+        expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_in).with(anything, account.id)
       end
     end
 
@@ -86,12 +81,7 @@ RSpec.describe 'Deleting profile images' do
         account.reload
 
         expect(account.header).to_not exist
-      end
-
-      it 'queues up an account update distribution' do
-        delete '/api/v1/profile/header', headers: headers
-
-        expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_async).with(account.id)
+        expect(ActivityPub::UpdateDistributionWorker).to have_received(:perform_in).with(anything, account.id)
       end
     end
   end