Merge branch '262086-schedule-user-unset-status' into 'master'

Schedule user status cleanup [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!53620

app/models/user_status.rb

@@ -7,6 +7,16 @@ class UserStatus < ApplicationRecord
 
   DEFAULT_EMOJI = 'speech_balloon'
 
+  CLEAR_STATUS_QUICK_OPTIONS = {
+    '30_minutes' => 30.minutes,
+    '3_hours'    => 3.hours,
+    '8_hours'    => 8.hours,
+    '1_day'      => 1.day,
+    '3_days'     => 3.days,
+    '7_days'     => 7.days,
+    '30_days'    => 30.days
+  }.freeze
+
   belongs_to :user
 
   enum availability: { not_set: 0, busy: 1 }
@@ -15,5 +25,11 @@ class UserStatus < ApplicationRecord
   validates :emoji, inclusion: { in: Gitlab::Emoji.emojis_names }
   validates :message, length: { maximum: 100 }, allow_blank: true
 
+  scope :scheduled_for_cleanup, -> { where(arel_table[:clear_status_at].lteq(Time.current)) }
+
   cache_markdown_field :message, pipeline: :emoji
+
+  def clear_status_after=(value)
+    self.clear_status_at = CLEAR_STATUS_QUICK_OPTIONS[value]&.from_now
+  end
 end

app/services/users/batch_status_cleaner_service.rb

+# frozen_string_literal: true
+
+module Users
+  class BatchStatusCleanerService
+    BATCH_SIZE = 100.freeze
+
+    # Cleanup BATCH_SIZE user_statuses records
+    # rubocop: disable CodeReuse/ActiveRecord
+    def self.execute(batch_size: BATCH_SIZE)
+      scope = UserStatus
+        .select(:user_id)
+        .scheduled_for_cleanup
+        .lock('FOR UPDATE SKIP LOCKED')
+        .limit(batch_size)
+
+      deleted_rows = UserStatus.where(user_id: scope).delete_all
+
+      { deleted_rows: deleted_rows }
+    end
+    # rubocop: enable CodeReuse/ActiveRecord
+  end
+end

app/workers/all_queues.yml

@@ -467,6 +467,14 @@
   :weight: 1
   :idempotent: true
   :tags: []
+- :name: cronjob:user_status_cleanup_batch
+  :feature_category: :users
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent: true
+  :tags: []
 - :name: cronjob:users_create_statistics
   :feature_category: :users
   :has_external_dependencies:

app/workers/user_status_cleanup/batch_worker.rb

+# frozen_string_literal: true
+
+module UserStatusCleanup
+  # This worker will run every minute to look for user status records to clean up.
+  class BatchWorker
+    include ApplicationWorker
+    # rubocop:disable Scalability/CronWorkerContext
+    include CronjobQueue
+    # rubocop:enable Scalability/CronWorkerContext
+
+    feature_category :users
+
+    idempotent!
+
+    # Avoid running too many UPDATE queries at once
+    MAX_RUNTIME = 30.seconds
+
+    def perform
+      return unless UserStatus.scheduled_for_cleanup.exists?
+
+      start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC)
+
+      loop do
+        result = Users::BatchStatusCleanerService.execute
+        break if result[:deleted_rows] < Users::BatchStatusCleanerService::BATCH_SIZE
+
+        current_time = Process.clock_gettime(Process::CLOCK_MONOTONIC)
+
+        break if (current_time - start_time) > MAX_RUNTIME
+      end
+    end
+  end
+end

changelogs/unreleased/262086-schedule-user-unset-status.yml

+---
+title: Add clear_status_at column to user_status table
+merge_request: 53620
+author:
+type: other

config/feature_flags/development/clear_status_with_quick_options.yml

+---
+name: clear_status_with_quick_options
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/53620
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/320777
+milestone: '13.9'
+type: development
+group: group::optimize
+default_enabled: false

config/initializers/1_settings.rb

@@ -553,6 +553,9 @@ Settings.cron_jobs['schedule_merge_request_cleanup_refs_worker']['job_class'] =
 Settings.cron_jobs['manage_evidence_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['manage_evidence_worker']['cron'] ||= '0 * * * *'
 Settings.cron_jobs['manage_evidence_worker']['job_class'] = 'Releases::ManageEvidenceWorker'
+Settings.cron_jobs['user_status_cleanup_batch_worker'] ||= Settingslogic.new({})
+Settings.cron_jobs['user_status_cleanup_batch_worker']['cron'] ||= '* * * * *'
+Settings.cron_jobs['user_status_cleanup_batch_worker']['job_class'] = 'UserStatusCleanup::BatchWorker'
 
 Gitlab.com do
   Settings.cron_jobs['namespaces_in_product_marketing_emails_worker'] ||= Settingslogic.new({})

db/migrate/20210208125050_add_status_expires_at_to_user_statuses.rb

+# frozen_string_literal: true
+
+class AddStatusExpiresAtToUserStatuses < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_column(:user_statuses, :clear_status_at, :datetime_with_timezone, null: true)
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_column(:user_statuses, :clear_status_at)
+    end
+  end
+end

db/migrate/20210208125248_add_index_on_user_statuses_status_expires_at.rb

+# frozen_string_literal: true
+
+class AddIndexOnUserStatusesStatusExpiresAt < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INDEX_NAME = 'index_user_statuses_on_clear_status_at_not_null'
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index(:user_statuses, :clear_status_at, name: INDEX_NAME, where: 'clear_status_at IS NOT NULL')
+  end
+
+  def down
+    remove_concurrent_index_by_name(:user_statuses, INDEX_NAME)
+  end
+end

db/schema_migrations/20210208125050

+b9200d6c754f7c450ba0c718171806e8f4f9720d870e532f4800640ca707f24f
\ No newline at end of file

db/schema_migrations/20210208125248

+3a7fb1b7959f09b9ba464253a72d52bcb744e7f78aac4f44e1d9201fa3c8387d
\ No newline at end of file

db/structure.sql

@@ -17842,7 +17842,8 @@ CREATE TABLE user_statuses (
     emoji character varying DEFAULT 'speech_balloon'::character varying NOT NULL,
     message character varying(100),
     message_html character varying,
-    availability smallint DEFAULT 0 NOT NULL
+    availability smallint DEFAULT 0 NOT NULL,
+    clear_status_at timestamp with time zone
 );
 
 CREATE SEQUENCE user_statuses_user_id_seq
@@ -23521,6 +23522,8 @@ CREATE INDEX index_user_preferences_on_gitpod_enabled ON user_preferences USING
 
 CREATE UNIQUE INDEX index_user_preferences_on_user_id ON user_preferences USING btree (user_id);
 
+CREATE INDEX index_user_statuses_on_clear_status_at_not_null ON user_statuses USING btree (clear_status_at) WHERE (clear_status_at IS NOT NULL);
+
 CREATE INDEX index_user_statuses_on_user_id ON user_statuses USING btree (user_id);
 
 CREATE UNIQUE INDEX index_user_synced_attributes_metadata_on_user_id ON user_synced_attributes_metadata USING btree (user_id);

lib/api/entities/user_status.rb

@@ -9,6 +9,7 @@ module API
       expose :message_html do |entity|
         MarkupHelper.markdown_field(entity, :message)
       end
+      expose :clear_status_at
     end
   end
 end

lib/api/users.rb

@@ -1004,11 +1004,15 @@ module API
         optional :emoji, type: String, desc: "The emoji to set on the status"
         optional :message, type: String, desc: "The status message to set"
         optional :availability, type: String, desc: "The availability of user to set"
+        optional :clear_status_after, type: String, desc: "Automatically clear emoji, message and availability fields after a certain time", values: UserStatus::CLEAR_STATUS_QUICK_OPTIONS.keys
       end
       put "status", feature_category: :users do
         forbidden! unless can?(current_user, :update_user_status, current_user)
 
-        if ::Users::SetStatusService.new(current_user, declared_params).execute
+        update_params = declared_params
+        update_params.delete(:clear_status_after) if Feature.disabled?(:clear_status_with_quick_options, current_user)
+
+        if ::Users::SetStatusService.new(current_user, update_params).execute
           present current_user.status, with: Entities::UserStatus
         else
           render_validation_error!(current_user.status)

spec/models/user_status_spec.rb

@@ -17,4 +17,34 @@ RSpec.describe UserStatus do
 
     expect { status.user.destroy }.to change { described_class.count }.from(1).to(0)
   end
+
+  describe '#clear_status_after=' do
+    it 'sets clear_status_at' do
+      status = build(:user_status)
+
+      freeze_time do
+        status.clear_status_after = '8_hours'
+
+        expect(status.clear_status_at).to be_like_time(8.hours.from_now)
+      end
+    end
+
+    it 'unsets clear_status_at' do
+      status = build(:user_status, clear_status_at: 8.hours.from_now)
+
+      status.clear_status_after = nil
+
+      expect(status.clear_status_at).to be_nil
+    end
+
+    context 'when unknown clear status is given' do
+      it 'unsets clear_status_at' do
+        status = build(:user_status, clear_status_at: 8.hours.from_now)
+
+        status.clear_status_after = 'unknown'
+
+        expect(status.clear_status_at).to be_nil
+      end
+    end
+  end
 end

spec/requests/api/users_spec.rb

@@ -2866,6 +2866,47 @@ RSpec.describe API::Users do
       expect(response).to have_gitlab_http_status(:success)
       expect(user.reload.status).to be_nil
     end
+
+    context 'when clear_status_after is given' do
+      it 'sets the clear_status_at column' do
+        freeze_time do
+          expected_clear_status_at = 3.hours.from_now
+
+          put api('/user/status', user), params: { emoji: 'smirk', message: 'hello world', clear_status_after: '3_hours' }
+
+          expect(response).to have_gitlab_http_status(:success)
+          expect(user.status.reload.clear_status_at).to be_within(1.minute).of(expected_clear_status_at)
+          expect(Time.parse(json_response["clear_status_at"])).to be_within(1.minute).of(expected_clear_status_at)
+        end
+      end
+
+      it 'unsets the clear_status_at column' do
+        user.create_status!(clear_status_at: 5.hours.ago)
+
+        put api('/user/status', user), params: { emoji: 'smirk', message: 'hello world', clear_status_after: nil }
+
+        expect(response).to have_gitlab_http_status(:success)
+        expect(user.status.reload.clear_status_at).to be_nil
+      end
+
+      it 'raises error when unknown status value is given' do
+        put api('/user/status', user), params: { emoji: 'smirk', message: 'hello world', clear_status_after: 'wrong' }
+
+        expect(response).to have_gitlab_http_status(:bad_request)
+      end
+
+      context 'when the clear_status_with_quick_options feature flag is disabled' do
+        before do
+          stub_feature_flags(clear_status_with_quick_options: false)
+        end
+
+        it 'does not persist clear_status_at' do
+          put api('/user/status', user), params: { emoji: 'smirk', message: 'hello world', clear_status_after: '3_hours' }
+
+          expect(user.status.reload.clear_status_at).to be_nil
+        end
+      end
+    end
   end
 
   describe 'POST /users/:user_id/personal_access_tokens' do

spec/services/users/batch_status_cleaner_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Users::BatchStatusCleanerService do
+  let_it_be(:user_status_1) { create(:user_status, emoji: 'coffee', message: 'msg1', clear_status_at: 1.year.ago) }
+  let_it_be(:user_status_2) { create(:user_status, emoji: 'coffee', message: 'msg1', clear_status_at: 1.year.from_now) }
+  let_it_be(:user_status_3) { create(:user_status, emoji: 'coffee', message: 'msg1', clear_status_at: 2.years.ago) }
+  let_it_be(:user_status_4) { create(:user_status, emoji: 'coffee', message: 'msg1') }
+
+  subject(:result) { described_class.execute }
+
+  it 'cleans up scheduled user statuses' do
+    expect(result[:deleted_rows]).to eq(2)
+
+    deleted_statuses = UserStatus.where(user_id: [user_status_1.user_id, user_status_3.user_id])
+    expect(deleted_statuses).to be_empty
+  end
+
+  it 'does not affect rows with future clear_status_at' do
+    expect { result }.not_to change { user_status_2.reload }
+  end
+
+  it 'does not affect rows without clear_status_at' do
+    expect { result }.not_to change { user_status_4.reload }
+  end
+
+  describe 'batch_size' do
+    it 'clears status in batches' do
+      result = described_class.execute(batch_size: 1)
+
+      expect(result[:deleted_rows]).to eq(1)
+
+      result = described_class.execute(batch_size: 1)
+
+      expect(result[:deleted_rows]).to eq(1)
+
+      result = described_class.execute(batch_size: 1)
+
+      expect(result[:deleted_rows]).to eq(0)
+    end
+  end
+end

spec/workers/user_status_cleanup/batch_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe UserStatusCleanup::BatchWorker do
+  include_examples 'an idempotent worker' do
+    subject do
+      perform_multiple([], worker: described_class.new)
+    end
+  end
+
+  describe '#perform' do
+    subject(:run_worker) { described_class.new.perform }
+
+    context 'when no records are scheduled for cleanup' do
+      let(:user_status) { create(:user_status) }
+
+      it 'does nothing' do
+        expect { run_worker }.not_to change { user_status.reload }
+      end
+    end
+
+    it 'cleans up the records' do
+      user_status_1 = create(:user_status, clear_status_at: 1.year.ago)
+      user_status_2 = create(:user_status, clear_status_at: 2.years.ago)
+
+      run_worker
+
+      deleted_statuses = UserStatus.where(user_id: [user_status_1.user_id, user_status_2.user_id])
+      expect(deleted_statuses).to be_empty
+    end
+  end
+end