From 23ac76555bc0ddb3ca5fad07d6c0ba08e67fbdaf Mon Sep 17 00:00:00 2001
From: Rob Zolkos <rob@zolkos.com>
Date: Mon, 9 Mar 2026 17:37:56 -0400
Subject: [PATCH] Validate and normalize auto-postpone period to days (#2672)

* Validate and normalize auto-postpone period to days

- Add Entropy::AUTO_POSTPONE_PERIODS and validate auto_postpone_period against the allowed set
- Introduce auto_postpone_period_in_days for forms and entropy update endpoints
- Fall back to index 0 in knob partial when current value isn't in options
- Remove entropy_auto_close_options helper in favor of model constant
- Update tests to use allowed period values

* Use auto_postpone_period_in_days for JSON entropy updates

The JSON API was accepting auto_postpone_period (seconds) which
bypassed the days normalization and validation. Switch to
auto_postpone_period_in_days consistently and add explicit
wrap_parameters so flat JSON params are wrapped correctly for
the virtual attribute.

* Default to account or 30-day fallback when knob value is invalid

* Address PR review feedback for entropy validation

- Fall back to first knob option (index 0) when persisted value isn't in
  the allowed set, preventing nil index errors for legacy values
- Use integer division (1.day.to_i) for consistent day calculations

* Default to account entropy period instead of first option for knob fallback

* Test that default auto-postpone period is in the allowed periods

* Return 422 instead of 500 for invalid entropy auto-postpone values

Rescue ActiveRecord::RecordInvalid in entropy controllers so invalid
auto_postpone_period_in_days values return 422 Unprocessable Entity
instead of raising a 500.

* Fix NoMethodError when board entropy falls back to account default

Use container.account.entropy.auto_postpone_period_in_days instead of
container.account.auto_postpone_period_in_days since Account doesn't
delegate that method.

* Test board entropy fallback to account default for invalid periods
---
 .../account/entropies_controller.rb           |  6 +++-
 .../boards/entropies_controller.rb            |  6 ++--
 app/controllers/boards_controller.rb          |  2 +-
 app/helpers/entropy_helper.rb                 |  4 ---
 app/models/board/auto_postponing.rb           |  4 +--
 app/models/board/entropic.rb                  |  8 +++--
 app/models/entropy.rb                         | 29 +++++++++++++++++++
 app/views/entropy/_auto_close.html.erb        |  5 ++--
 app/views/entropy/_knob.html.erb              |  5 ++--
 .../accounts/entropies_controller_test.rb     | 19 ++++++++----
 test/controllers/api/flat_json_params_test.rb |  8 ++---
 .../boards/entropies_controller_test.rb       | 19 ++++++++----
 test/controllers/boards_controller_test.rb    | 14 +++++++--
 test/models/card/entropic_test.rb             | 10 +++----
 test/models/entropy_test.rb                   | 15 ++++++++--
 15 files changed, 113 insertions(+), 41 deletions(-)

diff --git a/app/controllers/account/entropies_controller.rb b/app/controllers/account/entropies_controller.rb
index 7290c1e4e..327098ee2 100644
--- a/app/controllers/account/entropies_controller.rb
+++ b/app/controllers/account/entropies_controller.rb
@@ -1,4 +1,6 @@
 class Account::EntropiesController < ApplicationController
+  wrap_parameters :entropy, include: [ :auto_postpone_period_in_days ]
+
   before_action :ensure_admin
 
   def update
@@ -8,10 +10,12 @@ class Account::EntropiesController < ApplicationController
       format.html { redirect_to account_settings_path, notice: "Account updated" }
       format.json { head :no_content }
     end
+  rescue ActiveRecord::RecordInvalid
+    head :unprocessable_entity
   end
 
   private
     def entropy_params
-      params.expect(entropy: [ :auto_postpone_period ])
+      params.expect(entropy: [ :auto_postpone_period_in_days ])
     end
 end
diff --git a/app/controllers/boards/entropies_controller.rb b/app/controllers/boards/entropies_controller.rb
index 557adb2d9..132733aac 100644
--- a/app/controllers/boards/entropies_controller.rb
+++ b/app/controllers/boards/entropies_controller.rb
@@ -1,5 +1,5 @@
 class Boards::EntropiesController < ApplicationController
-  wrap_parameters :board
+  wrap_parameters :board, include: [ :auto_postpone_period_in_days ]
 
   include BoardScoped
 
@@ -12,10 +12,12 @@ class Boards::EntropiesController < ApplicationController
       format.turbo_stream
       format.json { head :no_content }
     end
+  rescue ActiveRecord::RecordInvalid
+    head :unprocessable_entity
   end
 
   private
     def entropy_params
-      params.expect(board: [ :auto_postpone_period ])
+      params.expect(board: [ :auto_postpone_period_in_days ])
     end
 end
diff --git a/app/controllers/boards_controller.rb b/app/controllers/boards_controller.rb
index 1e93a5758..8e63455f9 100644
--- a/app/controllers/boards_controller.rb
+++ b/app/controllers/boards_controller.rb
@@ -88,7 +88,7 @@ class BoardsController < ApplicationController
     end
 
     def board_params
-      params.expect(board: [ :name, :all_access, :auto_postpone_period, :public_description ])
+      params.expect(board: [ :name, :all_access, :auto_postpone_period_in_days, :public_description ])
     end
 
     def grantees
diff --git a/app/helpers/entropy_helper.rb b/app/helpers/entropy_helper.rb
index 4329dab88..7257ba481 100644
--- a/app/helpers/entropy_helper.rb
+++ b/app/helpers/entropy_helper.rb
@@ -1,8 +1,4 @@
 module EntropyHelper
-  def entropy_auto_close_options
-    [ 3, 7, 30, 90, 365, 11 ]
-  end
-
   def entropy_bubble_options_for(card)
     {
       daysBeforeReminder: card.entropy.days_before_reminder,
diff --git a/app/models/board/auto_postponing.rb b/app/models/board/auto_postponing.rb
index 4911c5e33..da53692b3 100644
--- a/app/models/board/auto_postponing.rb
+++ b/app/models/board/auto_postponing.rb
@@ -6,9 +6,7 @@ module Board::AutoPostponing
   end
 
   private
-    DEFAULT_AUTO_POSTPONE_PERIOD = 30.days
-
     def set_default_auto_postpone_period
-      self.auto_postpone_period ||= DEFAULT_AUTO_POSTPONE_PERIOD unless attribute_present?(:auto_postpone_period)
+      self.auto_postpone_period ||= Entropy::DEFAULT_AUTO_POSTPONE_PERIOD_IN_DAYS.days unless attribute_present?(:auto_postpone_period)
     end
 end
diff --git a/app/models/board/entropic.rb b/app/models/board/entropic.rb
index beee4883b..d3e48d6ea 100644
--- a/app/models/board/entropic.rb
+++ b/app/models/board/entropic.rb
@@ -2,7 +2,7 @@ module Board::Entropic
   extend ActiveSupport::Concern
 
   included do
-    delegate :auto_postpone_period, to: :entropy
+    delegate :auto_postpone_period, :auto_postpone_period_in_days, to: :entropy
     has_one :entropy, as: :container, dependent: :destroy
   end
 
@@ -12,6 +12,10 @@ module Board::Entropic
 
   def auto_postpone_period=(new_value)
     entropy ||= association(:entropy).reader || self.build_entropy
-    entropy.update auto_postpone_period: new_value
+    entropy.update! auto_postpone_period: new_value
+  end
+
+  def auto_postpone_period_in_days=(value)
+    self.auto_postpone_period = value.to_i.days.to_i
   end
 end
diff --git a/app/models/entropy.rb b/app/models/entropy.rb
index 9a93ba4e8..c896fc494 100644
--- a/app/models/entropy.rb
+++ b/app/models/entropy.rb
@@ -1,6 +1,35 @@
 class Entropy < ApplicationRecord
+  DEFAULT_AUTO_POSTPONE_PERIOD_IN_DAYS = 30
+  AUTO_POSTPONE_PERIODS_IN_DAYS = [ 3, 7, 30, 90, 365, 11 ].freeze
+  AUTO_POSTPONE_PERIODS_IN_SECONDS = AUTO_POSTPONE_PERIODS_IN_DAYS.map { |n| n.day.in_seconds }.freeze
+
   belongs_to :account, default: -> { container.account }
   belongs_to :container, polymorphic: true
 
+  validates :auto_postpone_period, inclusion: { in: AUTO_POSTPONE_PERIODS_IN_SECONDS }
+
   after_commit -> { container.cards.touch_all if container }
+
+  def auto_postpone_period_in_days
+    days = auto_postpone_period / 1.day.to_i
+
+    if days.in?(AUTO_POSTPONE_PERIODS_IN_DAYS)
+      days
+    else
+      default_auto_postpone_period_in_days
+    end
+  end
+
+  def auto_postpone_period_in_days=(new_value)
+    self.auto_postpone_period = new_value.to_i.days.to_i
+  end
+
+  private
+    def default_auto_postpone_period_in_days
+      if container.is_a?(Board)
+        container.account.entropy.auto_postpone_period_in_days
+      else
+        DEFAULT_AUTO_POSTPONE_PERIOD_IN_DAYS
+      end
+    end
 end
diff --git a/app/views/entropy/_auto_close.html.erb b/app/views/entropy/_auto_close.html.erb
index 6ccb5d205..a2e3fe0ab 100644
--- a/app/views/entropy/_auto_close.html.erb
+++ b/app/views/entropy/_auto_close.html.erb
@@ -6,8 +6,9 @@
     <%= form_with model: model, url: url, data: { controller: "form" } do |form| %>
       <%= render "entropy/knob",
             form: form,
-            name: :auto_postpone_period,
-            knob_options: entropy_auto_close_options,
+            name: :auto_postpone_period_in_days,
+            current_value: model.auto_postpone_period_in_days,
+            knob_options: Entropy::AUTO_POSTPONE_PERIODS_IN_DAYS,
             label: "Days until auto-close",
             disabled: disabled  %>
     <% end %>
diff --git a/app/views/entropy/_knob.html.erb b/app/views/entropy/_knob.html.erb
index 69bbb5fb0..ba17c6728 100644
--- a/app/views/entropy/_knob.html.erb
+++ b/app/views/entropy/_knob.html.erb
@@ -1,6 +1,5 @@
 <%
-  period = form.object.send(name)
-  current_index = knob_options.index(period.to_i / 1.day)
+  current_index = knob_options.index(current_value) || knob_options.index(Entropy::DEFAULT_AUTO_POSTPONE_PERIOD_IN_DAYS)
 %>
 
 <fieldset class="knob" data-controller="knob" data-knob-target="field" style="--knob-options: <%= knob_options.length %>; --knob-index: <%= current_index %>">
@@ -8,7 +7,7 @@
     <% knob_options.each_with_index do |value, index| %>
       <label class="knob__option" style="--i: <%= index %>">
         <%= form.radio_button name,
-              value.days,
+              value,
               data: {
                 action: "change->knob#optionChanged change->form#submit",
                 index: index,
diff --git a/test/controllers/accounts/entropies_controller_test.rb b/test/controllers/accounts/entropies_controller_test.rb
index 5903ed7d0..d00c0d768 100644
--- a/test/controllers/accounts/entropies_controller_test.rb
+++ b/test/controllers/accounts/entropies_controller_test.rb
@@ -6,24 +6,33 @@ class Account::EntropiesControllerTest < ActionDispatch::IntegrationTest
   end
 
   test "update" do
-    put account_entropy_path, params: { entropy: { auto_postpone_period: 1.day } }
+    put account_entropy_path, params: { entropy: { auto_postpone_period_in_days: 7 } }
 
-    assert_equal 1.day, entropies("37s_account").auto_postpone_period
+    assert_equal 7.days, entropies("37s_account").auto_postpone_period
 
     assert_redirected_to account_settings_path
   end
 
   test "update as JSON" do
-    put account_entropy_path, params: { entropy: { auto_postpone_period: 2.days } }, as: :json
+    put account_entropy_path, params: { entropy: { auto_postpone_period_in_days: 7 } }, as: :json
 
     assert_response :no_content
-    assert_equal 2.days, entropies("37s_account").reload.auto_postpone_period
+    assert_equal 7.days, entropies("37s_account").reload.auto_postpone_period
   end
 
   test "update requires admin" do
     logout_and_sign_in_as :david
 
-    put account_entropy_path, params: { entropy: { auto_postpone_period: 1.day } }
+    put account_entropy_path, params: { entropy: { auto_postpone_period_in_days: 7 } }
     assert_response :forbidden
   end
+
+  test "update rejects invalid auto_postpone_period" do
+    original_period = entropies("37s_account").auto_postpone_period
+
+    put account_entropy_path, params: { entropy: { auto_postpone_period_in_days: 1 } }
+
+    assert_response :unprocessable_entity
+    assert_equal original_period, entropies("37s_account").reload.auto_postpone_period
+  end
 end
diff --git a/test/controllers/api/flat_json_params_test.rb b/test/controllers/api/flat_json_params_test.rb
index 4efba5209..22c51c365 100644
--- a/test/controllers/api/flat_json_params_test.rb
+++ b/test/controllers/api/flat_json_params_test.rb
@@ -39,17 +39,17 @@ class FlatJsonParamsTest < ActionDispatch::IntegrationTest
   test "update board entropy with flat JSON" do
     board = boards(:writebook)
 
-    put board_entropy_path(board), params: { auto_postpone_period: 99.days }, as: :json
+    put board_entropy_path(board), params: { auto_postpone_period_in_days: 90 }, as: :json
 
     assert_response :no_content
-    assert_equal 99.days, board.entropy.reload.auto_postpone_period
+    assert_equal 90.days, board.entropy.reload.auto_postpone_period
   end
 
   test "update account entropy with flat JSON" do
-    put account_entropy_path, params: { auto_postpone_period: 2.days }, as: :json
+    put account_entropy_path, params: { auto_postpone_period_in_days: 7 }, as: :json
 
     assert_response :no_content
-    assert_equal 2.days, Current.account.entropy.reload.auto_postpone_period
+    assert_equal 7.days, Current.account.entropy.reload.auto_postpone_period
   end
 
   test "create push subscription with flat JSON" do
diff --git a/test/controllers/boards/entropies_controller_test.rb b/test/controllers/boards/entropies_controller_test.rb
index c0ec089a5..e0fd4fa52 100644
--- a/test/controllers/boards/entropies_controller_test.rb
+++ b/test/controllers/boards/entropies_controller_test.rb
@@ -8,19 +8,19 @@ class Boards::EntropiesControllerTest < ActionDispatch::IntegrationTest
 
   test "update" do
     assert_no_difference -> { Current.account.entropy.reload.auto_postpone_period } do
-      put board_entropy_path(@board, format: :turbo_stream), params: { board: { auto_postpone_period: 123.days } }
+      put board_entropy_path(@board, format: :turbo_stream), params: { board: { auto_postpone_period_in_days: 90 } }
 
-      assert_equal 123.days, @board.entropy.reload.auto_postpone_period
+      assert_equal 90.days, @board.entropy.reload.auto_postpone_period
 
       assert_turbo_stream action: :replace, target: dom_id(@board, :entropy)
     end
   end
 
   test "update as JSON" do
-    put board_entropy_path(@board), params: { board: { auto_postpone_period: 99.days } }, as: :json
+    put board_entropy_path(@board), params: { board: { auto_postpone_period_in_days: 90 } }, as: :json
 
     assert_response :no_content
-    assert_equal 99.days, @board.entropy.reload.auto_postpone_period
+    assert_equal 90.days, @board.entropy.reload.auto_postpone_period
   end
 
   test "update requires board admin permission" do
@@ -28,9 +28,18 @@ class Boards::EntropiesControllerTest < ActionDispatch::IntegrationTest
 
     original_period = @board.entropy.auto_postpone_period
 
-    put board_entropy_path(@board, format: :turbo_stream), params: { board: { auto_postpone_period: 1.day } }
+    put board_entropy_path(@board, format: :turbo_stream), params: { board: { auto_postpone_period_in_days: 7 } }
 
     assert_response :forbidden
     assert_equal original_period, @board.entropy.reload.auto_postpone_period
   end
+
+  test "update rejects invalid auto_postpone_period" do
+    original_period = @board.entropy.auto_postpone_period
+
+    put board_entropy_path(@board, format: :turbo_stream), params: { board: { auto_postpone_period_in_days: 1 } }
+
+    assert_response :unprocessable_entity
+    assert_equal original_period, @board.entropy.reload.auto_postpone_period
+  end
 end
diff --git a/test/controllers/boards_controller_test.rb b/test/controllers/boards_controller_test.rb
index fedd9d188..915310108 100644
--- a/test/controllers/boards_controller_test.rb
+++ b/test/controllers/boards_controller_test.rb
@@ -41,12 +41,22 @@ class BoardsControllerTest < ActionDispatch::IntegrationTest
     assert_response :success
   end
 
+  test "edit renders 11-day auto-close option last on the knob" do
+    get edit_board_path(boards(:writebook))
+    assert_response :success
+
+    assert_select "input[type=radio][name='board[auto_postpone_period_in_days]']" do |options|
+      assert_equal Entropy::AUTO_POSTPONE_PERIODS_IN_DAYS.map(&:to_s), options.map { |option| option["value"] }
+      assert_equal "11", options.last["value"]
+    end
+  end
+
   test "update" do
     patch board_path(boards(:writebook)), params: {
       board: {
         name: "Writebook bugs",
         all_access: false,
-        auto_postpone_period: 1.day
+        auto_postpone_period_in_days: 7
       },
       user_ids: users(:kevin, :jz).pluck(:id)
     }
@@ -54,7 +64,7 @@ class BoardsControllerTest < ActionDispatch::IntegrationTest
     assert_redirected_to edit_board_path(boards(:writebook))
     assert_equal "Writebook bugs", boards(:writebook).reload.name
     assert_equal users(:kevin, :jz).sort, boards(:writebook).users.sort
-    assert_equal 1.day, entropies(:writebook_board).auto_postpone_period
+    assert_equal 7.days, entropies(:writebook_board).auto_postpone_period
     assert_not boards(:writebook).all_access?
   end
 
diff --git a/test/models/card/entropic_test.rb b/test/models/card/entropic_test.rb
index 9707eabf4..fe5dfd806 100644
--- a/test/models/card/entropic_test.rb
+++ b/test/models/card/entropic_test.rb
@@ -9,17 +9,17 @@ class Card::EntropicTest < ActiveSupport::TestCase
     freeze_time
 
     entropies(:writebook_board).destroy
-    entropies("37s_account").reload.update! auto_postpone_period: 456.days
+    entropies("37s_account").reload.update! auto_postpone_period: 365.days
     cards(:layout).update! last_active_at: 2.day.ago
-    assert_equal (456 - 2).days.from_now, cards(:layout).entropy.auto_clean_at
+    assert_equal (365 - 2).days.from_now, cards(:layout).entropy.auto_clean_at
   end
 
   test "auto_postpone_at infers the period from the board when present" do
     freeze_time
 
-    entropies(:writebook_board).update! auto_postpone_period: 123.days
+    entropies(:writebook_board).update! auto_postpone_period: 90.days
     cards(:layout).update! last_active_at: 2.day.ago
-    assert_equal (123 - 2).days.from_now, cards(:layout).entropy.auto_clean_at
+    assert_equal (90 - 2).days.from_now, cards(:layout).entropy.auto_clean_at
   end
 
   test "setting auto_postpone_period in the board without entropy will create it, without affecting the account entropy" do
@@ -27,7 +27,7 @@ class Card::EntropicTest < ActiveSupport::TestCase
     original_period = account_entropy.auto_postpone_period
 
     entropies(:writebook_board).destroy
-    boards(:writebook).update! auto_postpone_period: 999.days
+    boards(:writebook).update! auto_postpone_period: 365.days
 
     assert_equal original_period, account_entropy.reload.auto_postpone_period
   end
diff --git a/test/models/entropy_test.rb b/test/models/entropy_test.rb
index 0648bbd3b..98abb0af0 100644
--- a/test/models/entropy_test.rb
+++ b/test/models/entropy_test.rb
@@ -3,15 +3,26 @@ require "test_helper"
 class Entropy::Test < ActiveSupport::TestCase
   test "touch cards when entropy changes for board" do
     assert_changes -> { boards(:writebook).cards.first.updated_at } do
-      boards(:writebook).entropy.update!(auto_postpone_period: 15.days)
+      boards(:writebook).entropy.update!(auto_postpone_period: 7.days)
     end
   end
 
+  test "default auto-postpone period is included in allowed periods" do
+    assert_includes Entropy::AUTO_POSTPONE_PERIODS_IN_DAYS, Entropy::DEFAULT_AUTO_POSTPONE_PERIOD_IN_DAYS
+  end
+
+  test "board entropy falls back to account entropy period when value is invalid" do
+    board = boards(:writebook)
+    board.entropy.update_column(:auto_postpone_period, 999.days.to_i)
+
+    assert_equal Current.account.entropy.auto_postpone_period_in_days, board.entropy.auto_postpone_period_in_days
+  end
+
   test "touch cards when entropy changes for account container" do
     account = Current.account
 
     assert_changes -> { account.cards.first.updated_at } do
-      boards(:writebook).entropy.update!(auto_postpone_period: 15.days)
+      boards(:writebook).entropy.update!(auto_postpone_period: 7.days)
     end
   end
 end