Merge branch '331927-revert-temp-all-records-to-expiration-emails' into 'master'

Revert temporary change for sending expiration email for all SSH keys

See merge request gitlab-org/gitlab!73185

app/models/key.rb

@@ -46,7 +46,7 @@ class Key < ApplicationRecord
   scope :order_last_used_at_desc, -> { reorder(::Gitlab::Database.nulls_last_order('last_used_at', 'DESC')) }
 
   # Date is set specifically in this scope to improve query time.
-  scope :expired_and_not_notified, -> { where(["date(expires_at AT TIME ZONE 'UTC') BETWEEN '2000-01-01' AND CURRENT_DATE AND expiry_notification_delivered_at IS NULL"]) }
+  scope :expired_today_and_not_notified, -> { where(["date(expires_at AT TIME ZONE 'UTC') = CURRENT_DATE AND expiry_notification_delivered_at IS NULL"]) }
   scope :expiring_soon_and_not_notified, -> { where(["date(expires_at AT TIME ZONE 'UTC') > CURRENT_DATE AND date(expires_at AT TIME ZONE 'UTC') < ? AND before_expiry_notification_delivered_at IS NULL", DAYS_TO_EXPIRE.days.from_now.to_date]) }
 
   def self.regular_keys

app/models/user.rb

@@ -123,7 +123,7 @@ class User < ApplicationRecord
 
   # Profile
   has_many :keys, -> { regular_keys }, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
-  has_many :expired_and_unnotified_keys, -> { expired_and_not_notified }, class_name: 'Key'
+  has_many :expired_today_and_unnotified_keys, -> { expired_today_and_not_notified }, class_name: 'Key'
   has_many :expiring_soon_and_unnotified_keys, -> { expiring_soon_and_not_notified }, class_name: 'Key'
   has_many :deploy_keys, -> { where(type: 'DeployKey') }, dependent: :nullify # rubocop:disable Cop/ActiveRecordDependent
   has_many :group_deploy_keys

app/workers/ssh_keys/expired_notification_worker.rb

@@ -29,7 +29,7 @@ module SshKeys
         )
       ])
 
-      scope = Key.expired_and_not_notified.order(order)
+      scope = Key.expired_today_and_not_notified.order(order)
 
       iterator = Gitlab::Pagination::Keyset::Iterator.new(scope: scope, use_union_optimization: true)
       iterator.each_batch(of: BATCH_SIZE) do |relation|
@@ -37,7 +37,7 @@ module SshKeys
 
         users.each do |user|
           with_context(user: user) do
-            Keys::ExpiryNotificationService.new(user, { keys: user.expired_and_unnotified_keys, expiring_soon: false }).execute
+            Keys::ExpiryNotificationService.new(user, { keys: user.expired_today_and_unnotified_keys, expiring_soon: false }).execute
           end
         end
       end

config/initializers/1_settings.rb

@@ -571,7 +571,7 @@ 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'
 Settings.cron_jobs['ssh_keys_expired_notification_worker'] ||= Settingslogic.new({})
-Settings.cron_jobs['ssh_keys_expired_notification_worker']['cron'] ||= '0 2 * * *'
+Settings.cron_jobs['ssh_keys_expired_notification_worker']['cron'] ||= '0 2,14 * * *'
 Settings.cron_jobs['ssh_keys_expired_notification_worker']['job_class'] = 'SshKeys::ExpiredNotificationWorker'
 Settings.cron_jobs['namespaces_in_product_marketing_emails_worker'] ||= Settingslogic.new({})
 Settings.cron_jobs['namespaces_in_product_marketing_emails_worker']['cron'] ||= '0 16 * * *'

db/post_migrate/20211027112901_drop_index_keys_on_expires_at_and_before_expiry_notification_undelivered.rb

+# frozen_string_literal: true
+
+class DropIndexKeysOnExpiresAtAndBeforeExpiryNotificationUndelivered < Gitlab::Database::Migration[1.0]
+  DOWNTIME = false
+  INDEX_NAME = 'index_keys_on_expires_at_and_expiry_notification_undelivered'
+  disable_ddl_transaction!
+
+  def up
+    remove_concurrent_index_by_name(:keys, INDEX_NAME)
+  end
+
+  def down
+    add_concurrent_index :keys,
+                         "date(timezone('UTC', expires_at)), expiry_notification_delivered_at",
+                         where: 'expiry_notification_delivered_at IS NULL', name: INDEX_NAME
+  end
+end

db/schema_migrations/20211027112901

+83c1d699e5de98007ef815b5f9dfbc9a644c6289bf86832af1d5b8a6d3d83659
\ No newline at end of file

db/structure.sql

@@ -25727,8 +25727,6 @@ CREATE INDEX index_jira_imports_on_user_id ON jira_imports USING btree (user_id)
 
 CREATE INDEX index_jira_tracker_data_on_service_id ON jira_tracker_data USING btree (service_id);
 
-CREATE INDEX index_keys_on_expires_at_and_expiry_notification_undelivered ON keys USING btree (date(timezone('UTC'::text, expires_at)), expiry_notification_delivered_at) WHERE (expiry_notification_delivered_at IS NULL);
-
 CREATE INDEX index_keys_on_expires_at_and_id ON keys USING btree (date(timezone('UTC'::text, expires_at)), id) WHERE (expiry_notification_delivered_at IS NULL);
 
 CREATE UNIQUE INDEX index_keys_on_fingerprint ON keys USING btree (fingerprint);

spec/models/key_spec.rb

@@ -85,9 +85,9 @@ RSpec.describe Key, :mailer do
       let_it_be(:expiring_soon_notified) { create(:key, expires_at: 4.days.from_now, user: user, before_expiry_notification_delivered_at: Time.current) }
       let_it_be(:future_expiry) { create(:key, expires_at: 1.month.from_now, user: user) }
 
-      describe '.expired_and_not_notified' do
+      describe '.expired_today_and_not_notified' do
         it 'returns keys that expire today and in the past' do
-          expect(described_class.expired_and_not_notified).to contain_exactly(expired_today_not_notified, expired_yesterday)
+          expect(described_class.expired_today_and_not_notified).to contain_exactly(expired_today_not_notified)
         end
       end
 

spec/models/user_spec.rb

@@ -98,7 +98,7 @@ RSpec.describe User do
     it { is_expected.to have_many(:group_members) }
     it { is_expected.to have_many(:groups) }
     it { is_expected.to have_many(:keys).dependent(:destroy) }
-    it { is_expected.to have_many(:expired_and_unnotified_keys) }
+    it { is_expected.to have_many(:expired_today_and_unnotified_keys) }
     it { is_expected.to have_many(:deploy_keys).dependent(:nullify) }
     it { is_expected.to have_many(:group_deploy_keys) }
     it { is_expected.to have_many(:events).dependent(:delete_all) }

spec/workers/ssh_keys/expired_notification_worker_spec.rb

@@ -20,7 +20,7 @@ RSpec.describe SshKeys::ExpiredNotificationWorker, type: :worker do
         stub_const("SshKeys::ExpiredNotificationWorker::BATCH_SIZE", 5)
       end
 
-      let_it_be_with_reload(:keys) { create_list(:key, 20, expires_at: 3.days.ago, user: user) }
+      let_it_be_with_reload(:keys) { create_list(:key, 20, expires_at: Time.current, user: user) }
 
       it 'updates all keys regardless of batch size' do
         worker.perform
@@ -54,8 +54,8 @@ RSpec.describe SshKeys::ExpiredNotificationWorker, type: :worker do
     context 'when key has expired in the past' do
       let_it_be(:expired_past) { create(:key, expires_at: 1.day.ago, user: user) }
 
-      it 'does update notified column' do
-        expect { worker.perform }.to change { expired_past.reload.expiry_notification_delivered_at }
+      it 'does not update notified column' do
+        expect { worker.perform }.not_to change { expired_past.reload.expiry_notification_delivered_at }
       end
 
       context 'when key has already been notified of expiration' do