put a max cap on allocated_time so we do not run into an integer range error

This commit is contained in:
Klaus Zanders
2026-06-11 10:02:06 +02:00
parent 6b276f2f33
commit 9065a2d85b
4 changed files with 44 additions and 2 deletions
@@ -115,12 +115,13 @@ module ResourceAllocations
end
# An allocation without a user yet (filter placeholder or lost principal)
# shows a person-add icon instead of an avatar.
# shows a person-add icon instead of an avatar. Sized to match the avatar so
# the leading column keeps the same width and the row stays aligned.
def leading_visual
if allocation.principal
Primer::OpenProject::AvatarWithFallback.new(size: AVATAR_SIZE, **avatar_options)
else
Primer::Beta::Octicon.new(icon: :"person-add", color: :muted, "aria-hidden": true)
Primer::Beta::Octicon.new(icon: :"person-add", size: :medium, color: :muted, "aria-hidden": true)
end
end
@@ -31,6 +31,11 @@
class ResourceAllocation < ApplicationRecord
ALLOWED_ENTITY_TYPES = %w[WorkPackage].freeze
# `allocated_time` is stored in minutes in an integer column. Cap it at 5000
# hours so an absurdly large input is rejected with a validation error rather
# than overflowing the column and raising ActiveModel::RangeError on save.
MAX_ALLOCATED_TIME = (5000.hours / 1.minute).to_i
belongs_to :entity, polymorphic: true, optional: false
belongs_to :principal, class_name: "User", optional: true, inverse_of: :resource_allocations
belongs_to :requested_by, class_name: "User", optional: true
@@ -119,6 +124,8 @@ class ResourceAllocation < ApplicationRecord
presence: true,
numericality: { only_integer: true, greater_than: 0 }
validate :allocated_time_within_limit
validates :entity_type,
inclusion: { in: ALLOWED_ENTITY_TYPES },
allow_blank: true
@@ -207,6 +214,17 @@ class ResourceAllocation < ApplicationRecord
private
# Capped above to keep `allocated_time` within the integer column. The field is
# entered in hours, so the message reports the limit in the same duration
# format rather than the raw minutes the column stores.
def allocated_time_within_limit
return if allocated_time.blank?
return if allocated_time <= MAX_ALLOCATED_TIME
errors.add(:allocated_time, :less_than_or_equal_to,
count: DurationConverter.output(MAX_ALLOCATED_TIME / 60.0))
end
def starts_before_entity?
entity_start_date.present? && start_date.present? && start_date < entity_start_date
end
@@ -401,6 +401,17 @@ RSpec.describe ResourceAllocation do
allocation.allocated_time = 1
expect(allocation).to be_valid
end
it "is valid at the upper cap" do
allocation.allocated_time = described_class::MAX_ALLOCATED_TIME
expect(allocation).to be_valid
end
it "is invalid (rather than overflowing the column) above the cap" do
allocation.allocated_time = described_class::MAX_ALLOCATED_TIME + 1
expect(allocation).not_to be_valid
expect(allocation.errors.symbols_for(:allocated_time)).to include(:less_than_or_equal_to)
end
end
describe "date range" do
@@ -525,6 +525,18 @@ RSpec.describe "ResourceAllocations requests",
expect(response).to have_http_status(:unprocessable_entity)
expect(allocation.reload.allocated_time).to eq(600)
end
# Regression: an absurdly large value used to overflow the integer column
# and raise ActiveModel::RangeError (500) instead of failing validation.
it "rejects a value above the maximum with an hours-formatted message" do
perform(allocated_hours: "999999999999h")
expect(response).to have_http_status(:unprocessable_entity)
expect(allocation.reload.allocated_time).to eq(600)
expect(response.body).to include(
DurationConverter.output(ResourceAllocation::MAX_ALLOCATED_TIME / 60.0)
)
end
end
context "when the update would overbook the assigned user" do