diff --git a/app/contracts/settings/update_contract.rb b/app/contracts/settings/update_contract.rb index 001349680164..6f75f67b94db 100644 --- a/app/contracts/settings/update_contract.rb +++ b/app/contracts/settings/update_contract.rb @@ -31,5 +31,20 @@ module Settings class UpdateContract < ::BaseContract include RequiresAdminGuard + + validate :journal_aggregation_time_minutes_range + + private + + def journal_aggregation_time_minutes_range + return unless model.respond_to?(:[]) + + value = model[:journal_aggregation_time_minutes] + return if value.nil? + + unless (0..3600).include?(value.to_i) + errors.add(:journal_aggregation_time_minutes, :inclusion) + end + end end end diff --git a/app/forms/admin/settings/aggregation_settings_form.rb b/app/forms/admin/settings/aggregation_settings_form.rb index b3466f9b7cc3..12db844c89f1 100644 --- a/app/forms/admin/settings/aggregation_settings_form.rb +++ b/app/forms/admin/settings/aggregation_settings_form.rb @@ -37,6 +37,7 @@ class AggregationSettingsForm < ApplicationForm f.text_field name: :journal_aggregation_time_minutes, type: :number, min: 0, + max: 3600, input_width: :medium, trailing_visual: { text: { text: I18n.t("datetime.units.minute_abbreviated", count: 2) } } diff --git a/app/services/settings/update_service.rb b/app/services/settings/update_service.rb index 848d42f00461..796c59a5f45f 100644 --- a/app/services/settings/update_service.rb +++ b/app/services/settings/update_service.rb @@ -34,6 +34,15 @@ def initialize(user:) contract_class: Settings::UpdateContract) end + def validate_contract(call) + contract = Settings::UpdateContract.new(params, user) + unless contract.valid? + call.success = false + call.errors = contract.errors + end + call + end + def persist(call) params.each do |name, value| set_setting_value(name, value) diff --git a/app/workers/notifications/workflow_job.rb b/app/workers/notifications/workflow_job.rb index 120ed38fe696..1b3c3453461f 100644 --- a/app/workers/notifications/workflow_job.rb +++ b/app/workers/notifications/workflow_job.rb @@ -82,7 +82,7 @@ class Notifications::WorkflowJob < ApplicationJob end state :send_mails, - wait: -> { Setting.journal_aggregation_time_minutes.to_i.minutes } do |*notification_ids| + wait: -> { [Setting.journal_aggregation_time_minutes.to_i, 3600].min.minutes } do |*notification_ids| next unless notification_ids Notification diff --git a/spec/contracts/settings/update_contract_spec.rb b/spec/contracts/settings/update_contract_spec.rb index 7b347c7eb225..1c322a8ed392 100644 --- a/spec/contracts/settings/update_contract_spec.rb +++ b/spec/contracts/settings/update_contract_spec.rb @@ -39,4 +39,30 @@ end it_behaves_like "contract is valid for active admins and invalid for regular users" + + describe "journal_aggregation_time_minutes validation" do + let(:current_user) { build_stubbed(:admin) } + + [0, 5, 3600].each do |valid_value| + context "with value #{valid_value}" do + let(:contract) { described_class.new({ journal_aggregation_time_minutes: valid_value.to_s }, current_user) } + + it_behaves_like "contract is valid" + end + end + + [3601, 9_999_999, -1].each do |invalid_value| + context "with value #{invalid_value}" do + let(:contract) { described_class.new({ journal_aggregation_time_minutes: invalid_value.to_s }, current_user) } + + it_behaves_like "contract is invalid", journal_aggregation_time_minutes: :inclusion + end + end + + context "when not present in params" do + let(:contract) { described_class.new({}, current_user) } + + it_behaves_like "contract is valid" + end + end end