From eb8d4314aa4b59724756b1c31325ab39aedf63ae Mon Sep 17 00:00:00 2001 From: "Ben Sheldon [he/him]" Date: Mon, 18 Mar 2024 15:45:05 +0100 Subject: [PATCH] Add `enabled_by_default: false` as option for cron configuration --- README.md | 5 +-- app/models/good_job/cron_entry.rb | 7 +++- app/models/good_job/execution.rb | 4 +-- app/models/good_job/setting.rb | 38 +++++++++++++++------ config/locales/de.yml | 2 +- config/locales/en.yml | 2 +- config/locales/es.yml | 2 +- config/locales/fr.yml | 6 ++-- config/locales/it.yml | 2 +- config/locales/nl.yml | 2 +- config/locales/ru.yml | 2 +- spec/app/models/good_job/cron_entry_spec.rb | 38 +++++++++++++++++++++ 12 files changed, 86 insertions(+), 24 deletions(-) diff --git a/README.md b/README.md index 7631d8ea0..18f14dc9e 100644 --- a/README.md +++ b/README.md @@ -627,9 +627,10 @@ config.good_job.cron = { set: { priority: -10 }, # additional Active Job properties; can also be a lambda/proc e.g. `-> { { priority: [1,2].sample } }` description: "Something helpful", # optional description that appears in Dashboard }, - another_task: { + production_task: { cron: "0 0,12 * * *", - class: "AnotherJob", + class: "ProductionJob", + enabled_by_default: -> { Rails.env.production? } # Only enable in production, otherwise can be enabled manually through Dashboard }, complex_schedule: { class: "ComplexScheduleJob", diff --git a/app/models/good_job/cron_entry.rb b/app/models/good_job/cron_entry.rb index 2a44ac7c7..f8db6bc6c 100644 --- a/app/models/good_job/cron_entry.rb +++ b/app/models/good_job/cron_entry.rb @@ -74,7 +74,7 @@ def next_at(previously_at: nil) def enabled? return true unless GoodJob::Setting.migrated? - GoodJob::Setting.cron_key_enabled?(key) + GoodJob::Setting.cron_key_enabled?(key, default: enabled_by_default?) end def enable @@ -132,6 +132,11 @@ def last_job_at private + def enabled_by_default? + value = params.fetch(:enabled_by_default, true) + value.respond_to?(:call) ? value.call : value + end + def cron params.fetch(:cron) end diff --git a/app/models/good_job/execution.rb b/app/models/good_job/execution.rb index 56520fdb0..3564490fc 100644 --- a/app/models/good_job/execution.rb +++ b/app/models/good_job/execution.rb @@ -399,7 +399,7 @@ def perform job_class: job_class, queue_name: queue_name, serialized_params: serialized_params, - scheduled_at: (scheduled_at || created_at), + scheduled_at: scheduled_at || created_at, created_at: job_performed_at ) update!(performed_at: job_performed_at, executions_count: ((executions_count || 0) + 1)) @@ -482,7 +482,7 @@ def perform end end - preserve_unhandled = (result.unhandled_error && (GoodJob.retry_on_unhandled_error || GoodJob.preserve_job_records == :on_unhandled_error)) + preserve_unhandled = result.unhandled_error && (GoodJob.retry_on_unhandled_error || GoodJob.preserve_job_records == :on_unhandled_error) if GoodJob.preserve_job_records == true || reenqueued || preserve_unhandled || cron_key.present? if discrete_execution transaction do diff --git a/app/models/good_job/setting.rb b/app/models/good_job/setting.rb index 8416f87d4..fe7515227 100644 --- a/app/models/good_job/setting.rb +++ b/app/models/good_job/setting.rb @@ -2,29 +2,47 @@ module GoodJob class Setting < BaseRecord + CRON_KEYS_ENABLED = "cron_keys_enabled" CRON_KEYS_DISABLED = "cron_keys_disabled" self.table_name = 'good_job_settings' - def self.cron_key_enabled?(key) - cron_disabled = find_by(key: CRON_KEYS_DISABLED)&.value || [] - cron_disabled.exclude?(key.to_s) + def self.cron_key_enabled?(key, default: true) + if default + cron_disabled = find_by(key: CRON_KEYS_DISABLED)&.value || [] + cron_disabled.exclude?(key.to_s) + else + cron_enabled = find_by(key: CRON_KEYS_ENABLED)&.value || [] + cron_enabled.include?(key.to_s) + end end def self.cron_key_enable(key) - setting = GoodJob::Setting.find_by(key: CRON_KEYS_DISABLED) - return unless setting&.value&.include?(key.to_s) + enabled_setting = find_or_initialize_by(key: CRON_KEYS_ENABLED) do |record| + record.value = [] + end + enabled_setting.value << key unless enabled_setting.value.include?(key) + enabled_setting.save! - setting.value.delete(key.to_s) - setting.save! + disabled_setting = GoodJob::Setting.find_by(key: CRON_KEYS_DISABLED) + return unless disabled_setting&.value&.include?(key.to_s) + + disabled_setting.value.delete(key.to_s) + disabled_setting.save! end def self.cron_key_disable(key) - setting = find_or_initialize_by(key: CRON_KEYS_DISABLED) do |record| + enabled_setting = GoodJob::Setting.find_by(key: CRON_KEYS_ENABLED) + if enabled_setting&.value&.include?(key.to_s) + enabled_setting.value.delete(key.to_s) + enabled_setting.save! + end + + disabled_setting = find_or_initialize_by(key: CRON_KEYS_DISABLED) do |record| record.value = [] end - setting.value << key - setting.save! + disabled_setting.value << key unless disabled_setting.value.include?(key) + disabled_setting.save! end end end diff --git a/config/locales/de.yml b/config/locales/de.yml index 93b6df187..6a629aa0b 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -34,7 +34,7 @@ de: cron_entries: actions: confirm_disable: Möchten Sie diesen Cron-Eintrag wirklich deaktivieren? - confirm_enable: Möchten Sie diesen Cron-Eintrag wirklich bestätigen? + confirm_enable: Möchten Sie diesen Cron-Eintrag wirklich aktivieren? confirm_enqueue: Möchten Sie diesen Cron-Eintrag wirklich in die Warteschlange stellen? disable: Cron-Eintrag deaktivieren enable: Cron-Eintrag aktivieren diff --git a/config/locales/en.yml b/config/locales/en.yml index e9a04bad9..4d1cfe4f7 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -34,7 +34,7 @@ en: cron_entries: actions: confirm_disable: Are you sure you want to disable this cron entry? - confirm_enable: Are you sure you want to confirm this cron entry? + confirm_enable: Are you sure you want to enable this cron entry? confirm_enqueue: Are you sure you want to enqueue this cron entry? disable: Disable cron entry enable: Enable cron entry diff --git a/config/locales/es.yml b/config/locales/es.yml index e4b67d343..d0a8c66ca 100644 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -34,7 +34,7 @@ es: cron_entries: actions: confirm_disable: "¿Estás seguro que querés deshabilitar esta tarea programada?" - confirm_enable: "¿Estás seguro que querés confirmar esta tarea programada?" + confirm_enable: "¿Estás seguro que querés habilitar esta tarea programada?" confirm_enqueue: "¿Estás seguro que querés encolar esta tarea programada?" disable: Deshabilitar tarea programada enable: Habilitar tarea programada diff --git a/config/locales/fr.yml b/config/locales/fr.yml index f36d59807..47aa256c1 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -33,9 +33,9 @@ fr: no_batches_found: Aucun lot trouvé. cron_entries: actions: - confirm_disable: Voulez-vous vraiment désactiver cette entrée cron ? - confirm_enable: Voulez-vous vraiment activer cette entrée cron ? - confirm_enqueue: Voulez-vous vraiment mettre en file d'attente cette entrée cron ? + confirm_disable: Voulez-vous vraiment désactiver cette entrée cron? + confirm_enable: Voulez-vous vraiment activer cette entrée cron? + confirm_enqueue: Voulez-vous vraiment mettre en file d'attente cette entrée cron? disable: Désactiver l'entrée cron enable: Activer l'entrée cron enqueue: Mettre en file d'attente l'entrée cron maintenant diff --git a/config/locales/it.yml b/config/locales/it.yml index f602188b8..455789ed2 100644 --- a/config/locales/it.yml +++ b/config/locales/it.yml @@ -34,7 +34,7 @@ it: cron_entries: actions: confirm_disable: Sei sicuro di voler disabilitare questa voce cron? - confirm_enable: Sei sicuro di voler confermare questa voce cron? + confirm_enable: Sei sicuro di voler abilita questa voce cron? confirm_enqueue: Sei sicuro di voler mettere in coda questa voce cron? disable: Disabilita voce cron enable: Abilita voce cron diff --git a/config/locales/nl.yml b/config/locales/nl.yml index a6713a26b..093665819 100644 --- a/config/locales/nl.yml +++ b/config/locales/nl.yml @@ -34,7 +34,7 @@ nl: cron_entries: actions: confirm_disable: Weet u zeker dat u deze cron-vermelding wilt uitschakelen? - confirm_enable: Weet u zeker dat u deze cron-invoer wilt bevestigen? + confirm_enable: Weet u zeker dat u deze cron-invoer wilt inschakelen? confirm_enqueue: Weet u zeker dat u deze cron-vermelding in de wachtrij wilt plaatsen? disable: Schakel cron-invoer uit enable: Schakel cron-invoer in diff --git a/config/locales/ru.yml b/config/locales/ru.yml index 764e95edc..53082b19e 100644 --- a/config/locales/ru.yml +++ b/config/locales/ru.yml @@ -34,7 +34,7 @@ ru: cron_entries: actions: confirm_disable: Вы уверены, что хотите отключить эту задачу cron? - confirm_enable: Вы уверены, что хотите подтвердить эту задачу cron? + confirm_enable: Вы уверены, что хотите включить это задание cron? confirm_enqueue: Вы уверены, что хотите поставить эту задачу cron в очередь? disable: Отключить задачу cron enable: Включить задачу cron diff --git a/spec/app/models/good_job/cron_entry_spec.rb b/spec/app/models/good_job/cron_entry_spec.rb index e49d5c317..4215d5bb5 100644 --- a/spec/app/models/good_job/cron_entry_spec.rb +++ b/spec/app/models/good_job/cron_entry_spec.rb @@ -70,6 +70,44 @@ def perform(meaning, name:) end end + describe '#enabled' do + it 'is enabled by default' do + expect(entry).to be_enabled + end + + it 'can be enabled and disabled' do + entry.disable + expect(entry).not_to be_enabled + + entry.enable + expect(entry).to be_enabled + end + + context "when enabled_by_default=false" do + let(:params) { super().merge(enabled_by_default: false) } + + it 'is disabled by default' do + expect(entry).not_to be_enabled + end + + it 'can be enabled and disabled' do + entry.enable + expect(entry).to be_enabled + + entry.disable + expect(entry).not_to be_enabled + end + end + + context 'when a lambda' do + let(:params) { super().merge(enabled_by_default: -> { false }) } + + it 'is disabled by default' do + expect(entry).not_to be_enabled + end + end + end + describe 'display_schedule' do it 'returns the cron expression' do expect(entry.display_schedule).to eq('* * * * *')