[STC-356] Disallow setting journal aggregation period to >2 hours

app/contracts/settings/update_params_contract.rb

@@ -0,0 +1,47 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+module Settings
+  class UpdateParamsContract < ::ParamsContract
+    validate :journal_aggregation_time_minutes_is_within_bounds
+
+    protected
+
+    def journal_aggregation_time_minutes_is_within_bounds
+      value = params[:journal_aggregation_time_minutes]
+      return if value.nil?
+
+      allowed = Settings::Definition[:journal_aggregation_time_minutes].allowed
+      unless allowed.cover?(value.to_i)
+        errors.add(:journal_aggregation_time_minutes, :inclusion)
+      end
+    end
+  end
+end

app/forms/admin/settings/aggregation_settings_form.rb

@@ -34,9 +34,11 @@ module Admin
       include Redmine::I18n
 
       settings_form do |f|
+        allowed = ::Settings::Definition[:journal_aggregation_time_minutes].allowed
         f.text_field name: :journal_aggregation_time_minutes,
                      type: :number,
-                     min: 0,
+                     min: allowed.min,
+                     max: allowed.max,
                      input_width: :medium,
                      trailing_visual: { text: { text: I18n.t("datetime.units.minute_abbreviated", count: 2) } }
 

app/forms/admin/settings/aggregation_settings_form/journal_aggregation_time_minutes_caption.html.erb

@@ -1,5 +1,9 @@
 <%= render(Primer::Beta::Text.new(tag: :p)) do %>
-  <%= link_translate("admin.journal_aggregation.caption", links: { webhook_link: url_helpers.admin_outgoing_webhooks_path }) %>
+  <%= link_translate(
+        "admin.journal_aggregation.caption",
+        i18n_args: { max: ::Settings::Definition[:journal_aggregation_time_minutes].allowed.max },
+        links: { webhook_link: url_helpers.admin_outgoing_webhooks_path }
+      ) %>
 <% end %>
 <%= render(Primer::OpenProject::InlineMessage.new(scheme: :warning, size: :small)) do %>
   <%= render(Primer::Beta::Text.new(tag: :p)) do %>

app/services/settings/update_service.rb

@@ -34,6 +34,13 @@ class Settings::UpdateService < BaseServices::BaseContracted
           contract_class: Settings::UpdateContract)
   end
 
+  def validate_params
+    contract = Settings::UpdateParamsContract.new(model, user, params:)
+    ServiceResult.new success: contract.valid?,
+                      errors: contract.errors,
+                      result: model
+  end
+
   def persist(call)
     params.each do |name, value|
       set_setting_value(name, value)

config/constants/settings/definition.rb

@@ -697,7 +697,8 @@ module Settings
         default: 7
       },
       journal_aggregation_time_minutes: {
-        default: 5
+        default: 5,
+        allowed: 0..120
       },
       ldap_force_no_page: {
         description: "Force LDAP to respond as a single page, in case paged responses do not work with your server.",

config/locales/en.yml

@@ -138,7 +138,7 @@ en:
     jemalloc_allocator: Jemalloc memory allocator
     journal_aggregation:
       caption: >
-        User actions on a work package (changing description, status, values, or writing comments) are grouped if performed within this period. It also controls notification and [webhook](webhook_link) delays.
+        User actions on a work package (changing description, status, values, or writing comments) are grouped if performed within this period. It also controls notification and [webhook](webhook_link) delays. The maximum is %{max} minutes.
     import:
       title: "Import"
     jira:

spec/contracts/settings/update_params_contract_spec.rb

@@ -0,0 +1,65 @@
+# frozen_string_literal: true
+
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+require "spec_helper"
+require "contracts/shared/model_contract_shared_context"
+
+RSpec.describe Settings::UpdateParamsContract do
+  include_context "ModelContract shared context"
+
+  let(:current_user) { build_stubbed(:admin) }
+  let(:contract) do
+    described_class.new(nil, current_user, params:)
+  end
+
+  describe "journal_aggregation_time_minutes validation" do
+    [0, 5, 120].each do |valid_value|
+      context "with value #{valid_value}" do
+        let(:params) { { journal_aggregation_time_minutes: valid_value.to_s } }
+
+        it_behaves_like "contract is valid"
+      end
+    end
+
+    [121, 9_999_999, -1].each do |invalid_value|
+      context "with value #{invalid_value}" do
+        let(:params) { { journal_aggregation_time_minutes: invalid_value.to_s } }
+
+        it_behaves_like "contract is invalid", journal_aggregation_time_minutes: :inclusion
+      end
+    end
+
+    context "when not present in params" do
+      let(:params) { {} }
+
+      it_behaves_like "contract is valid"
+    end
+  end
+end